[syzbot] KASAN: null-ptr-deref Read in __free_pages

[syzbot]

Hello,

syzbot found the following issue on:

HEAD commit: 56e337f2cf13 Revert "gpio: Revert regression in sysfs-gpio..
git tree: upstream
console output: https://syzkaller.appspot.com/x/log.txt?x=17e0a8c5700000
kernel config: https://syzkaller.appspot.com/x/.config?x=d35f9bc6884af6c9
dashboard link: https://syzkaller.appspot.com/bug?extid=d55757faa9b80590767b
compiler: gcc (Debian 10.2.1-6) 10.2.1 20210110, GNU ld (GNU Binutils for Debian) 2.35.2
syz repro: https://syzkaller.appspot.com/x/repro.syz?x=134e4e91700000
C reproducer: https://syzkaller.appspot.com/x/repro.c?x=127bd083700000

Bisection is inconclusive: the issue happens on the oldest tested release.

bisection log: https://syzkaller.appspot.com/x/bisect.txt?x=138601c5700000
final oops: https://syzkaller.appspot.com/x/report.txt?x=104601c5700000
console output: https://syzkaller.appspot.com/x/log.txt?x=178601c5700000

IMPORTANT: if you fix the issue, please add the following tag to the commit:
Reported-by: syzbot+d55757...@syzkaller.appspotmail.com

R10: 0000000000000000 R11: 0000000000000246 R12: 0000000000000001
R13: 00007ffc794ecea0 R14: 000000000000000d R15: 00007ff328525490
</TASK>
==================================================================
BUG: KASAN: null-ptr-deref in instrument_atomic_read include/linux/instrumented.h:71 [inline]
BUG: KASAN: null-ptr-deref in atomic_read include/linux/atomic/atomic-instrumented.h:27 [inline]
BUG: KASAN: null-ptr-deref in page_ref_count include/linux/page_ref.h:67 [inline]
BUG: KASAN: null-ptr-deref in put_page_testzero include/linux/mm.h:717 [inline]
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

CPU: 1 PID: 3599 Comm: syz-executor168 Not tainted 5.17.0-rc8-syzkaller-00003-g56e337f2cf13 #0
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/01/2011
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
RIP: 0033:0x7ff3284b2d59
Code: 28 00 00 00 75 05 48 83 c4 28 c3 e8 31 18 00 00 90 48 89 f8 48 89 f7 48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01 f0 ff ff 73 01 c3 48 c7 c1 c0 ff ff ff f7 d8 64 89 01 48
RSP: 002b:00007ffc794ece38 EFLAGS: 00000246 ORIG_RAX: 0000000000000010
RAX: ffffffffffffffda RBX: 0000000000000031 RCX: 00007ff3284b2d59
RDX: 0000000000000029 RSI: 0000000000005760 RDI: 0000000000000003
RBP: 00007ffc794ece40 R08: 0000000000000001 R09: 00007ffc794e0035
R10: 0000000000000000 R11: 0000000000000246 R12: 0000000000000001
R13: 00007ffc794ecea0 R14: 000000000000000d R15: 00007ff328525490
</TASK>
==================================================================


---
This report is generated by a bot. It may contain errors.
See https://goo.gl/tpsmEJ for more information about syzbot.
syzbot engineers can be reached at syzk...@googlegroups.com.

syzbot will keep track of this issue. See:
https://goo.gl/tpsmEJ#status for how to communicate with syzbot.
For information about bisection process see: https://goo.gl/tpsmEJ#bisection
syzbot can test patches for this issue, for details see:
https://goo.gl/tpsmEJ#testing-patches

[syzbot]

Hello,

syzbot tried to test the proposed patch but the build/boot failed:

kernel/watch_queue.c:365:13: error: invalid storage class for function '__put_watch_queue'
kernel/watch_queue.c:390:15: error: non-static declaration of 'put_watch_queue' follows static declaration
kernel/watch_queue.c:392:13: error: invalid storage class for function 'free_watch'
kernel/watch_queue.c:401:13: error: invalid storage class for function '__put_watch'
kernel/watch_queue.c:411:13: error: invalid storage class for function 'put_watch'
kernel/watch_queue.c:473:15: error: non-static declaration of 'add_watch_to_object' follows static declaration
kernel/watch_queue.c:554:15: error: non-static declaration of 'remove_watch_from_object' follows static declaration
kernel/watch_queue.c:644:15: error: non-static declaration of 'get_watch_queue' follows static declaration
kernel/watch_queue.c:664:1: error: expected declaration or statement at end of input
kernel/watch_queue.c:664: error: control reaches end of non-void function [-Werror=return-type]


Tested on:

commit: f443e374 Linux 5.17
git tree: upstream

dashboard link: https://syzkaller.appspot.com/bug?extid=d55757faa9b80590767b
compiler: gcc (Debian 10.2.1-6) 10.2.1 20210110, GNU ld (GNU Binutils for Debian) 2.35.2

patch: https://syzkaller.appspot.com/x/patch.diff?x=129f0625700000

[syzbot]

Hello,

syzbot has tested the proposed patch but the reproducer is still triggering an issue:
KASAN: null-ptr-deref Read in __free_pages

RBP: 00007ffd90d14ab0 R08: 0000000000000001 R09: 00007ffd90d10035

R10: 0000000000000000 R11: 0000000000000246 R12: 0000000000000001

R13: 00007ffd90d14b10 R14: 000000000000000d R15: 00007f2697250490

</TASK>
==================================================================
BUG: KASAN: null-ptr-deref in instrument_atomic_read include/linux/instrumented.h:71 [inline]
BUG: KASAN: null-ptr-deref in atomic_read include/linux/atomic/atomic-instrumented.h:27 [inline]
BUG: KASAN: null-ptr-deref in page_ref_count include/linux/page_ref.h:67 [inline]
BUG: KASAN: null-ptr-deref in put_page_testzero include/linux/mm.h:717 [inline]
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-executor333/4057

CPU: 0 PID: 4057 Comm: syz-executor333 Not tainted 5.17.0-syzkaller-00083-gf443e374ae13-dirty #0

Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/01/2011
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+0x4b1/0x640 kernel/watch_queue.c:276

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

RIP: 0033:0x7f26971ddd49

Code: 28 00 00 00 75 05 48 83 c4 28 c3 e8 31 18 00 00 90 48 89 f8 48 89 f7 48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01 f0 ff ff 73 01 c3 48 c7 c1 c0 ff ff ff f7 d8 64 89 01 48

RSP: 002b:00007ffd90d14aa8 EFLAGS: 00000246 ORIG_RAX: 0000000000000010
RAX: ffffffffffffffda RBX: 0000000000000031 RCX: 00007f26971ddd49

RDX: 0000000000000029 RSI: 0000000000005760 RDI: 0000000000000003

RBP: 00007ffd90d14ab0 R08: 0000000000000001 R09: 00007ffd90d10035

R10: 0000000000000000 R11: 0000000000000246 R12: 0000000000000001

R13: 00007ffd90d14b10 R14: 000000000000000d R15: 00007f2697250490
</TASK>
==================================================================

Tested on:

commit: f443e374 Linux 5.17
git tree: upstream

console output: https://syzkaller.appspot.com/x/log.txt?x=124166eb700000
kernel config: https://syzkaller.appspot.com/x/.config?x=19ca6f72fd444749

dashboard link: https://syzkaller.appspot.com/bug?extid=d55757faa9b80590767b
compiler: gcc (Debian 10.2.1-6) 10.2.1 20210110, GNU ld (GNU Binutils for Debian) 2.35.2

patch: https://syzkaller.appspot.com/x/patch.diff?x=109444dd700000

[syzbot]

Hello,

syzbot has tested the proposed patch and the reproducer did not trigger any issue:

Reported-and-tested-by: syzbot+d55757...@syzkaller.appspotmail.com

Tested on:

commit: f443e374 Linux 5.17
git tree: upstream

kernel config: https://syzkaller.appspot.com/x/.config?x=19ca6f72fd444749
dashboard link: https://syzkaller.appspot.com/bug?extid=d55757faa9b80590767b
compiler: gcc (Debian 10.2.1-6) 10.2.1 20210110, GNU ld (GNU Binutils for Debian) 2.35.2

patch: https://syzkaller.appspot.com/x/patch.diff?x=15ec554d700000

Note: testing is done by a robot and is best-effort only.

[David Howells]

It should be possible to just test for the pointer being NULL in the loop
before calling __free_pages() since the list was allocated with kcalloc().

David

#syz test: git://git.kernel.org/pub/scm/linux/kernel/git/dhowells/linux-fs.git 6d39b096627f0a1eb6e14f049d8ae3c93e0290f2

[syzbot]

Hello,

syzbot has tested the proposed patch and the reproducer did not trigger any issue:

Reported-and-tested-by: syzbot+d55757...@syzkaller.appspotmail.com

Tested on:

commit: 6d39b096 watch_queue: Fix NULL dereference in error cl..
git tree: git://git.kernel.org/pub/scm/linux/kernel/git/dhowells/linux-fs.git

kernel config: https://syzkaller.appspot.com/x/.config?x=19ca6f72fd444749
dashboard link: https://syzkaller.appspot.com/bug?extid=d55757faa9b80590767b
compiler: gcc (Debian 10.2.1-6) 10.2.1 20210110, GNU ld (GNU Binutils for Debian) 2.35.2

Note: no patches were applied.

[David Howells]

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+d55757...@syzkaller.appspotmail.com
Signed-off-by: David Howells <dhow...@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);

[David Howells]

Fabio M. De Francesco <fmdefr...@gmail.com> wrote:

> I cannot have any objection, it's up to you to decide :)

Of course you can object. That's kind of the point of a review;-)

How about the attached instead, then?

David
---
--- a/kernel/watch_queue.c
+++ b/kernel/watch_queue.c
@@ -271,9 +271,8 @@ long watch_queue_set_size(struct pipe_inode_info *pipe, unsigned int nr_notes)
return 0;

error_p:
- for (i = 0; i < nr_pages; i++)
- if (pages[i])
- __free_page(pages[i]);
+ while (--i >= 0)

[David Howells]

#syz test: git://git.kernel.org/pub/scm/linux/kernel/git/dhowells/linux-fs.git 733021c6d8607c5c5ab08bbe9b400d0da609185f

[syzbot]

Hello,

syzbot has tested the proposed patch and the reproducer did not trigger any issue:

Reported-and-tested-by: syzbot+d55757...@syzkaller.appspotmail.com

Tested on:

commit: 733021c6 watch_queue: Fix NULL dereference in error cl..