| Message ID | 19e36f90-d17e-ca6f-2c6e-11a712dc6d21@i-love.sakura.ne.jp (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Thank you. Please see comments below: On 14 June 2018 at 11:45, Tetsuo Handa <penguin-kernel@i-love.sakura.ne.jp> wrote: > (Which patch did you mean? Hmm, I paste both patches instead of waiting for your answer.) I only meant the BFS patch. > commit 0c54bbdb89992eeff50866b64366a1a0cbd0e6fb > Author: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp> > Date: Sat May 26 14:15:41 2018 +1000 > > bfs: add sanity check at bfs_fill_super() > > syzbot is reporting too large memory allocation at bfs_fill_super() [1]. > Since file system image is corrupted such that bfs_sb->s_start == 0, > bfs_fill_super() is trying to allocate 8MB of continuous memory. Fix this > by adding a sanity check on bfs_sb->s_start, __GFP_NOWARN and printf(). > > [1] https://syzkaller.appspot.com/bug?id=16a87c236b951351374a84c8a32f40edbc034e96 > > Link: http://lkml.kernel.org/r/1525862104-3407-1-git-send-email-penguin-kernel@I-love.SAKURA.ne.jp > Signed-off-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp> > Reported-by: syzbot <syzbot+71c6b5d68e91149fc8a4@syzkaller.appspotmail.com> > Reviewed-by: Andrew Morton <akpm@linux-foundation.org> > Cc: Tigran Aivazian <aivazian.tigran@gmail.com> > Cc: Matthew Wilcox <willy@infradead.org> > Signed-off-by: Andrew Morton <akpm@linux-foundation.org> > Signed-off-by: Stephen Rothwell <sfr@canb.auug.org.au> > > diff --git a/fs/bfs/inode.c b/fs/bfs/inode.c > index 9a69392..d81c148 100644 > --- a/fs/bfs/inode.c > +++ b/fs/bfs/inode.c > @@ -350,7 +350,8 @@ static int bfs_fill_super(struct super_block *s, void *data, int silent) > > s->s_magic = BFS_MAGIC; > > - if (le32_to_cpu(bfs_sb->s_start) > le32_to_cpu(bfs_sb->s_end)) { > + if (le32_to_cpu(bfs_sb->s_start) > le32_to_cpu(bfs_sb->s_end) || > + le32_to_cpu(bfs_sb->s_start) < BFS_BSIZE) { > printf("Superblock is corrupted\n"); > goto out1; > } Yes, this is acceptable. > @@ -359,9 +360,11 @@ static int bfs_fill_super(struct super_block *s, void *data, int silent) > sizeof(struct bfs_inode) > + BFS_ROOT_INO - 1; > imap_len = (info->si_lasti / 8) + 1; > - info->si_imap = kzalloc(imap_len, GFP_KERNEL); > - if (!info->si_imap) > + info->si_imap = kzalloc(imap_len, GFP_KERNEL | __GFP_NOWARN); > + if (!info->si_imap) { > + printf("Cannot allocate %u bytes\n", imap_len); > goto out1; > + } This is unnecessary. As Andrew Morton pointed out, if people set panic_on_warn then they do really want a panic on allocation failure warnings. Please update the patch with just the above sanity check for s_start < BFS_BSIZE. Kind regards, Tigran
diff --git a/fs/bfs/inode.c b/fs/bfs/inode.c index 9a69392..d81c148 100644 --- a/fs/bfs/inode.c +++ b/fs/bfs/inode.c @@ -350,7 +350,8 @@ static int bfs_fill_super(struct super_block *s, void *data, int silent) s->s_magic = BFS_MAGIC; - if (le32_to_cpu(bfs_sb->s_start) > le32_to_cpu(bfs_sb->s_end)) { + if (le32_to_cpu(bfs_sb->s_start) > le32_to_cpu(bfs_sb->s_end) || + le32_to_cpu(bfs_sb->s_start) < BFS_BSIZE) { printf("Superblock is corrupted\n"); goto out1; } @@ -359,9 +360,11 @@ static int bfs_fill_super(struct super_block *s, void *data, int silent) sizeof(struct bfs_inode) + BFS_ROOT_INO - 1; imap_len = (info->si_lasti / 8) + 1; - info->si_imap = kzalloc(imap_len, GFP_KERNEL); - if (!info->si_imap) + info->si_imap = kzalloc(imap_len, GFP_KERNEL | __GFP_NOWARN); + if (!info->si_imap) { + printf("Cannot allocate %u bytes\n", imap_len); goto out1; + } for (i = 0; i < BFS_ROOT_INO; i++) set_bit(i, info->si_imap); commit a343993c518ce252b62ec00ac06bccfb1d17129d Author: Bj旦rn T旦pel <bjorn.topel@intel.com> Date: Mon Jun 11 13:57:12 2018 +0200 xsk: silence warning on memory allocation failure syzkaller reported a warning from xdp_umem_pin_pages(): WARNING: CPU: 1 PID: 4537 at mm/slab_common.c:996 kmalloc_slab+0x56/0x70 mm/slab_common.c:996 ... __do_kmalloc mm/slab.c:3713 [inline] __kmalloc+0x25/0x760 mm/slab.c:3727 kmalloc_array include/linux/slab.h:634 [inline] kcalloc include/linux/slab.h:645 [inline] xdp_umem_pin_pages net/xdp/xdp_umem.c:205 [inline] xdp_umem_reg net/xdp/xdp_umem.c:318 [inline] xdp_umem_create+0x5c9/0x10f0 net/xdp/xdp_umem.c:349 xsk_setsockopt+0x443/0x550 net/xdp/xsk.c:531 __sys_setsockopt+0x1bd/0x390 net/socket.c:1935 __do_sys_setsockopt net/socket.c:1946 [inline] __se_sys_setsockopt net/socket.c:1943 [inline] __x64_sys_setsockopt+0xbe/0x150 net/socket.c:1943 do_syscall_64+0x1b1/0x800 arch/x86/entry/common.c:287 entry_SYSCALL_64_after_hwframe+0x49/0xbe This is a warning about attempting to allocate more than KMALLOC_MAX_SIZE memory. The request originates from userspace, and if the request is too big, the kernel is free to deny its allocation. In this patch, the failed allocation attempt is silenced with __GFP_NOWARN. Fixes: c0c77d8fb787 ("xsk: add user memory registration support sockopt") Reported-by: syzbot+4abadc5d69117b346506@syzkaller.appspotmail.com Signed-off-by: Bj旦rn T旦pel <bjorn.topel@intel.com> Signed-off-by: Daniel Borkmann <daniel@iogearbox.net> diff --git a/net/xdp/xdp_umem.c b/net/xdp/xdp_umem.c index b9ef487..f47abb4 100644 --- a/net/xdp/xdp_umem.c +++ b/net/xdp/xdp_umem.c @@ -204,7 +204,8 @@ static int xdp_umem_pin_pages(struct xdp_umem *umem) long npgs; int err; - umem->pgs = kcalloc(umem->npgs, sizeof(*umem->pgs), GFP_KERNEL); + umem->pgs = kcalloc(umem->npgs, sizeof(*umem->pgs), + GFP_KERNEL | __GFP_NOWARN); if (!umem->pgs) return -ENOMEM;