lkml.org 
[lkml]   [2022]   [Mar]   [21]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
SubjectRe: [syzbot] KASAN: null-ptr-deref Read in __free_pages
Date
On luned? 21 marzo 2022 09:46:51 CET David Howells wrote:
> Hi Fabio,
>
> How about using my patch instead? I wonder if __free_pages() should check for
> a NULL pages pointer...
>
> David
> ---
> watch_queue: Fix NULL dereference in error cleanup
>
> In watch_queue_set_size(), the error cleanup code doesn't take account of
> the fact that __free_page() can't handle a NULL pointer when trying to free
> up buffer pages that did get allocated.
>
> Fix this by only calling __free_page() on non-NULL pointers extracted from
> the list (the list was allocated by kcalloc() so was pre-cleared).
>
> Without the fix, this can lead to something like the following:
>
> BUG: KASAN: null-ptr-deref in __free_pages+0x1f/0x1b0 mm/page_alloc.c:5473
> Read of size 4 at addr 0000000000000034 by task syz-executor168/3599
> ...
> Call Trace:
> <TASK>
> __dump_stack lib/dump_stack.c:88 [inline]
> dump_stack_lvl+0xcd/0x134 lib/dump_stack.c:106
> __kasan_report mm/kasan/report.c:446 [inline]
> kasan_report.cold+0x66/0xdf mm/kasan/report.c:459
> check_region_inline mm/kasan/generic.c:183 [inline]
> kasan_check_range+0x13d/0x180 mm/kasan/generic.c:189
> instrument_atomic_read include/linux/instrumented.h:71 [inline]
> atomic_read include/linux/atomic/atomic-instrumented.h:27 [inline]
> page_ref_count include/linux/page_ref.h:67 [inline]
> put_page_testzero include/linux/mm.h:717 [inline]
> __free_pages+0x1f/0x1b0 mm/page_alloc.c:5473
> watch_queue_set_size+0x499/0x630 kernel/watch_queue.c:275
> pipe_ioctl+0xac/0x2b0 fs/pipe.c:632
> vfs_ioctl fs/ioctl.c:51 [inline]
> __do_sys_ioctl fs/ioctl.c:874 [inline]
> __se_sys_ioctl fs/ioctl.c:860 [inline]
> __x64_sys_ioctl+0x193/0x200 fs/ioctl.c:860
> do_syscall_x64 arch/x86/entry/common.c:50 [inline]
> do_syscall_64+0x35/0xb0 arch/x86/entry/common.c:80
> entry_SYSCALL_64_after_hwframe+0x44/0xae
>
> Fixes: c73be61cede5 ("pipe: Add general notification queue support")
> Reported-and-tested-by: syzbot+d55757faa9b80590767b@syzkaller.appspotmail.com
> Signed-off-by: David Howells <dhowells@redhat.com>
> ---
> kernel/watch_queue.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/kernel/watch_queue.c b/kernel/watch_queue.c
> index 00703444a219..09f35bab496e 100644
> --- a/kernel/watch_queue.c
> +++ b/kernel/watch_queue.c
> @@ -272,7 +272,8 @@ long watch_queue_set_size(struct pipe_inode_info *pipe, unsigned int nr_notes)
>
> error_p:
> for (i = 0; i < nr_pages; i++)
> - __free_page(pages[i]);
> + if (pages[i])
> + __free_page(pages[i]);
> kfree(pages);
> error:
> (void) account_pipe_buffers(pipe->user, nr_pages, pipe->nr_accounted);
>
>
I cannot have any objection, it's up to you to decide :)

I just noticed that you now have two tests in one loop:
the first is for the index less than an unnecessary high value:
the second is for pages[i] being NULL.

However, I have no other objections. Please use your patch instead.

Thanks,

Fabio M. De Francesco


\
 
 \ /
  Last update: 2022-03-21 10:06    [W:0.118 / U:0.776 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site