| Message ID | 20201116210530.26230-1-richard@nod.at |
|---|---|
| State | Accepted |
| Headers | show |
| Series | ubifs: wbuf: Don't leak kernel memory to flash | expand |
在 2020/11/17 5:05, Richard Weinberger 写道: > Write buffers use a kmalloc()'ed buffer, they can leak > up to seven bytes of kernel memory to flash if writes are not > aligned. > So use ubifs_pad() to fill these gaps with padding bytes. > This was never a problem while scanning because the scanner logic > manually aligns node lengths and skips over these gaps. > > Cc: <stable@vger.kernel.org> > Fixes: 1e51764a3c2ac05a2 ("UBIFS: add new flash file system") > Signed-off-by: Richard Weinberger <richard@nod.at> > --- > fs/ubifs/io.c | 13 +++++++++++-- > 1 file changed, 11 insertions(+), 2 deletions(-) > > diff --git a/fs/ubifs/io.c b/fs/ubifs/io.c > index 7e4bfaf2871f..eae9cf5a57b0 100644 > --- a/fs/ubifs/io.c > +++ b/fs/ubifs/io.c > @@ -319,7 +319,7 @@ void ubifs_pad(const struct ubifs_info *c, void *buf, int pad) > { > uint32_t crc; > > - ubifs_assert(c, pad >= 0 && !(pad & 7)); > + ubifs_assert(c, pad >= 0); > > if (pad >= UBIFS_PAD_NODE_SZ) { > struct ubifs_ch *ch = buf; > @@ -764,6 +764,10 @@ int ubifs_wbuf_write_nolock(struct ubifs_wbuf *wbuf, void *buf, int len) > * write-buffer. > */ > memcpy(wbuf->buf + wbuf->used, buf, len); > + if (aligned_len > len) { > + ubifs_assert(c, aligned_len - len < 8); > + ubifs_pad(c, wbuf->buf + wbuf->used + len, aligned_len - len); > + } > > if (aligned_len == wbuf->avail) { > dbg_io("flush jhead %s wbuf to LEB %d:%d", > @@ -856,13 +860,18 @@ int ubifs_wbuf_write_nolock(struct ubifs_wbuf *wbuf, void *buf, int len) > } > > spin_lock(&wbuf->lock); > - if (aligned_len) > + if (aligned_len) { > /* > * And now we have what's left and what does not take whole > * max. write unit, so write it to the write-buffer and we are > * done. > */ > memcpy(wbuf->buf, buf + written, len); > + if (aligned_len > len) { > + ubifs_assert(c, aligned_len - len < 8); > + ubifs_pad(c, wbuf->buf + len, aligned_len - len); > + } > + } > > if (c->leb_size - wbuf->offs >= c->max_write_size) > wbuf->size = c->max_write_size; > Reviewed-by: Zhihao Cheng <chengzhihao1@huawei.com>
On Tue, Nov 17, 2020 at 2:28 AM Zhihao Cheng <chengzhihao1@huawei.com> wrote: > > Reviewed-by: Zhihao Cheng <chengzhihao1@huawei.com> Thanks for reviewing, highly appreciated!
在 2020/11/17 16:43, Richard Weinberger 写道: > On Tue, Nov 17, 2020 at 2:28 AM Zhihao Cheng <chengzhihao1@huawei.com> wrote: >> >> Reviewed-by: Zhihao Cheng <chengzhihao1@huawei.com> > > Thanks for reviewing, highly appreciated! > You're welcome. Actually I've been following the linux-mtd. It's just that this patch isn't complicated, so I checked it. :-)
diff --git a/fs/ubifs/io.c b/fs/ubifs/io.c index 7e4bfaf2871f..eae9cf5a57b0 100644 --- a/fs/ubifs/io.c +++ b/fs/ubifs/io.c @@ -319,7 +319,7 @@ void ubifs_pad(const struct ubifs_info *c, void *buf, int pad) { uint32_t crc; - ubifs_assert(c, pad >= 0 && !(pad & 7)); + ubifs_assert(c, pad >= 0); if (pad >= UBIFS_PAD_NODE_SZ) { struct ubifs_ch *ch = buf; @@ -764,6 +764,10 @@ int ubifs_wbuf_write_nolock(struct ubifs_wbuf *wbuf, void *buf, int len) * write-buffer. */ memcpy(wbuf->buf + wbuf->used, buf, len); + if (aligned_len > len) { + ubifs_assert(c, aligned_len - len < 8); + ubifs_pad(c, wbuf->buf + wbuf->used + len, aligned_len - len); + } if (aligned_len == wbuf->avail) { dbg_io("flush jhead %s wbuf to LEB %d:%d", @@ -856,13 +860,18 @@ int ubifs_wbuf_write_nolock(struct ubifs_wbuf *wbuf, void *buf, int len) } spin_lock(&wbuf->lock); - if (aligned_len) + if (aligned_len) { /* * And now we have what's left and what does not take whole * max. write unit, so write it to the write-buffer and we are * done. */ memcpy(wbuf->buf, buf + written, len); + if (aligned_len > len) { + ubifs_assert(c, aligned_len - len < 8); + ubifs_pad(c, wbuf->buf + len, aligned_len - len); + } + } if (c->leb_size - wbuf->offs >= c->max_write_size) wbuf->size = c->max_write_size;
Write buffers use a kmalloc()'ed buffer, they can leak up to seven bytes of kernel memory to flash if writes are not aligned. So use ubifs_pad() to fill these gaps with padding bytes. This was never a problem while scanning because the scanner logic manually aligns node lengths and skips over these gaps. Cc: <stable@vger.kernel.org> Fixes: 1e51764a3c2ac05a2 ("UBIFS: add new flash file system") Signed-off-by: Richard Weinberger <richard@nod.at> --- fs/ubifs/io.c | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-)