[PATCH] fs/buffer: fix a NULL pointer dereference in drop_buffers()

[PATCH] fs/buffer: fix a NULL pointer dereference in drop_buffers()

[Liu Shixin]

syzbot found a null-ptr-deref by KASAN:

 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 buffer_busy fs/buffer.c:2856 [inline]
 BUG: KASAN: null-ptr-deref in drop_buffers+0x61/0x2f0 fs/buffer.c:2868
 Read of size 4 at addr 0000000000000060 by task syz-executor.5/24786

 CPU: 0 PID: 24786 Comm: syz-executor.5 Not tainted 6.0.0-syzkaller-09589-g55be6084c8e0 #0
 Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 09/22/2022
 Call Trace:
  <TASK>
  __dump_stack lib/dump_stack.c:88 [inline]
  dump_stack_lvl+0x1e3/0x2cb lib/dump_stack.c:106
  print_report+0xf1/0x220 mm/kasan/report.c:436
  kasan_report+0xfb/0x130 mm/kasan/report.c:495
  kasan_check_range+0x2a7/0x2e0 mm/kasan/generic.c:189
  instrument_atomic_read include/linux/instrumented.h:71 [inline]
  atomic_read include/linux/atomic/atomic-instrumented.h:27 [inline]
  buffer_busy fs/buffer.c:2856 [inline]
  drop_buffers+0x61/0x2f0 fs/buffer.c:2868
  try_to_free_buffers+0x2b1/0x640 fs/buffer.c:2898
[...]

We use folio_has_private() to decide whether call filemap_release_folio(),
which may call try_to_free_buffers() then. folio_has_private() return true
for both PG_private and PG_private_2. We should only call try_to_free_buffers()
for case PG_private. So we should recheck PG_private in try_to_free_buffers().

Reported-by: syzbot+fbdb4ec578ebdcfb9ed2@syzkaller.appspotmail.com
Fixes: 266cf658efcf ("FS-Cache: Recruit a page flags for cache management")
Signed-off-by: Liu Shixin <liushixin2@huawei.com>
---
 fs/buffer.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/fs/buffer.c b/fs/buffer.c
index d9c6d1fbb6dd..c302b578e437 100644
--- a/fs/buffer.c
+++ b/fs/buffer.c
@@ -2822,6 +2822,9 @@ bool try_to_free_buffers(struct folio *folio)
 	if (folio_test_writeback(folio))
 		return false;
 
+	if (!folio_test_private(folio))
+		return false;
+
 	if (mapping == NULL) {		/* can this still happen? */
 		ret = drop_buffers(folio, &buffers_to_free);
 		goto out;
-- 
2.25.1

Re: [PATCH] fs/buffer: fix a NULL pointer dereference in drop_buffers()

[Matthew Wilcox]

On Wed, Nov 09, 2022 at 05:50:18PM +0800, Liu Shixin wrote:
> syzbot found a null-ptr-deref by KASAN:
> 
>  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 buffer_busy fs/buffer.c:2856 [inline]
>  BUG: KASAN: null-ptr-deref in drop_buffers+0x61/0x2f0 fs/buffer.c:2868
>  Read of size 4 at addr 0000000000000060 by task syz-executor.5/24786
> 
>  CPU: 0 PID: 24786 Comm: syz-executor.5 Not tainted 6.0.0-syzkaller-09589-g55be6084c8e0 #0
>  Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 09/22/2022
>  Call Trace:
>   <TASK>
>   __dump_stack lib/dump_stack.c:88 [inline]
>   dump_stack_lvl+0x1e3/0x2cb lib/dump_stack.c:106
>   print_report+0xf1/0x220 mm/kasan/report.c:436
>   kasan_report+0xfb/0x130 mm/kasan/report.c:495
>   kasan_check_range+0x2a7/0x2e0 mm/kasan/generic.c:189
>   instrument_atomic_read include/linux/instrumented.h:71 [inline]
>   atomic_read include/linux/atomic/atomic-instrumented.h:27 [inline]
>   buffer_busy fs/buffer.c:2856 [inline]
>   drop_buffers+0x61/0x2f0 fs/buffer.c:2868
>   try_to_free_buffers+0x2b1/0x640 fs/buffer.c:2898
> [...]
> 
> We use folio_has_private() to decide whether call filemap_release_folio(),
> which may call try_to_free_buffers() then. folio_has_private() return true
> for both PG_private and PG_private_2. We should only call try_to_free_buffers()
> for case PG_private. So we should recheck PG_private in try_to_free_buffers().
> 
> Reported-by: syzbot+fbdb4ec578ebdcfb9ed2@syzkaller.appspotmail.com
> Fixes: 266cf658efcf ("FS-Cache: Recruit a page flags for cache management")

but this can only happen for a filesystem which uses both bufferheads
and PG_private_2.  afaik there aren't any of those in the tree.  so
this bug can't actually happen.

if you have your own filesystem that does, you need to submit it.

Re: [PATCH] fs/buffer: fix a NULL pointer dereference in drop_buffers()

[Liu Shixin]

On 2022/11/18 13:30, Matthew Wilcox wrote:
> On Wed, Nov 09, 2022 at 05:50:18PM +0800, Liu Shixin wrote:
>> syzbot found a null-ptr-deref by KASAN:
>>
>>  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 buffer_busy fs/buffer.c:2856 [inline]
>>  BUG: KASAN: null-ptr-deref in drop_buffers+0x61/0x2f0 fs/buffer.c:2868
>>  Read of size 4 at addr 0000000000000060 by task syz-executor.5/24786
>>
>>  CPU: 0 PID: 24786 Comm: syz-executor.5 Not tainted 6.0.0-syzkaller-09589-g55be6084c8e0 #0
>>  Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 09/22/2022
>>  Call Trace:
>>   <TASK>
>>   __dump_stack lib/dump_stack.c:88 [inline]
>>   dump_stack_lvl+0x1e3/0x2cb lib/dump_stack.c:106
>>   print_report+0xf1/0x220 mm/kasan/report.c:436
>>   kasan_report+0xfb/0x130 mm/kasan/report.c:495
>>   kasan_check_range+0x2a7/0x2e0 mm/kasan/generic.c:189
>>   instrument_atomic_read include/linux/instrumented.h:71 [inline]
>>   atomic_read include/linux/atomic/atomic-instrumented.h:27 [inline]
>>   buffer_busy fs/buffer.c:2856 [inline]
>>   drop_buffers+0x61/0x2f0 fs/buffer.c:2868
>>   try_to_free_buffers+0x2b1/0x640 fs/buffer.c:2898
>> [...]
>>
>> We use folio_has_private() to decide whether call filemap_release_folio(),
>> which may call try_to_free_buffers() then. folio_has_private() return true
>> for both PG_private and PG_private_2. We should only call try_to_free_buffers()
>> for case PG_private. So we should recheck PG_private in try_to_free_buffers().
>>
>> Reported-by: syzbot+fbdb4ec578ebdcfb9ed2@syzkaller.appspotmail.com
>> Fixes: 266cf658efcf ("FS-Cache: Recruit a page flags for cache management")
> but this can only happen for a filesystem which uses both bufferheads
> and PG_private_2.  afaik there aren't any of those in the tree.  so
> this bug can't actually happen.
>
> if you have your own filesystem that does, you need to submit it.
This null-ptr-deref is found by syzbot, not by my own filesystem. I review the related code and
found no other possible cause. There are lock protection all the place calling try_to_free_buffers().
So I only thought of this one possibility. I'm also trying to reproduce the problem but haven't
been successful.

If this can't actually happen, maybe I'm missing something when review the code. I'll keep trying
to see if I can reproduce the problem.

Thanks,
Liu Shixin
.

>
>
> .
>

Re: [PATCH] fs/buffer: fix a NULL pointer dereference in drop_buffers()

[Matthew Wilcox]

On Fri, Nov 18, 2022 at 03:54:54PM +0800, Liu Shixin wrote:
> On 2022/11/18 13:30, Matthew Wilcox wrote:
> > On Wed, Nov 09, 2022 at 05:50:18PM +0800, Liu Shixin wrote:
> >> syzbot found a null-ptr-deref by KASAN:
> >>
> >>  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 buffer_busy fs/buffer.c:2856 [inline]
> >>  BUG: KASAN: null-ptr-deref in drop_buffers+0x61/0x2f0 fs/buffer.c:2868
> >>  Read of size 4 at addr 0000000000000060 by task syz-executor.5/24786
> >>
> >>  CPU: 0 PID: 24786 Comm: syz-executor.5 Not tainted 6.0.0-syzkaller-09589-g55be6084c8e0 #0
> >>  Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 09/22/2022
> >>  Call Trace:
> >>   <TASK>
> >>   __dump_stack lib/dump_stack.c:88 [inline]
> >>   dump_stack_lvl+0x1e3/0x2cb lib/dump_stack.c:106
> >>   print_report+0xf1/0x220 mm/kasan/report.c:436
> >>   kasan_report+0xfb/0x130 mm/kasan/report.c:495
> >>   kasan_check_range+0x2a7/0x2e0 mm/kasan/generic.c:189
> >>   instrument_atomic_read include/linux/instrumented.h:71 [inline]
> >>   atomic_read include/linux/atomic/atomic-instrumented.h:27 [inline]
> >>   buffer_busy fs/buffer.c:2856 [inline]
> >>   drop_buffers+0x61/0x2f0 fs/buffer.c:2868
> >>   try_to_free_buffers+0x2b1/0x640 fs/buffer.c:2898
> >> [...]
> >>
> >> We use folio_has_private() to decide whether call filemap_release_folio(),
> >> which may call try_to_free_buffers() then. folio_has_private() return true
> >> for both PG_private and PG_private_2. We should only call try_to_free_buffers()
> >> for case PG_private. So we should recheck PG_private in try_to_free_buffers().
> >>
> >> Reported-by: syzbot+fbdb4ec578ebdcfb9ed2@syzkaller.appspotmail.com
> >> Fixes: 266cf658efcf ("FS-Cache: Recruit a page flags for cache management")
> > but this can only happen for a filesystem which uses both bufferheads
> > and PG_private_2.  afaik there aren't any of those in the tree.  so
> > this bug can't actually happen.
> >
> > if you have your own filesystem that does, you need to submit it.
> This null-ptr-deref is found by syzbot, not by my own filesystem. I review the related code and
> found no other possible cause. There are lock protection all the place calling try_to_free_buffers().
> So I only thought of this one possibility. I'm also trying to reproduce the problem but haven't
> been successful.
> 
> If this can't actually happen, maybe I'm missing something when review the code. I'll keep trying
> to see if I can reproduce the problem.

perhaps you could include more information, like the rest of the call
stack so we can see what filesystem is involved?

Re: [PATCH] fs/buffer: fix a NULL pointer dereference in drop_buffers()

[Liu Shixin]

On 2022/11/18 15:58, Matthew Wilcox wrote:
> On Fri, Nov 18, 2022 at 03:54:54PM +0800, Liu Shixin wrote:
>> On 2022/11/18 13:30, Matthew Wilcox wrote:
>>> On Wed, Nov 09, 2022 at 05:50:18PM +0800, Liu Shixin wrote:
>>>> syzbot found a null-ptr-deref by KASAN:
>>>>
>>>>  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 buffer_busy fs/buffer.c:2856 [inline]
>>>>  BUG: KASAN: null-ptr-deref in drop_buffers+0x61/0x2f0 fs/buffer.c:2868
>>>>  Read of size 4 at addr 0000000000000060 by task syz-executor.5/24786
>>>>
>>>>  CPU: 0 PID: 24786 Comm: syz-executor.5 Not tainted 6.0.0-syzkaller-09589-g55be6084c8e0 #0
>>>>  Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 09/22/2022
>>>>  Call Trace:
>>>>   <TASK>
>>>>   __dump_stack lib/dump_stack.c:88 [inline]
>>>>   dump_stack_lvl+0x1e3/0x2cb lib/dump_stack.c:106
>>>>   print_report+0xf1/0x220 mm/kasan/report.c:436
>>>>   kasan_report+0xfb/0x130 mm/kasan/report.c:495
>>>>   kasan_check_range+0x2a7/0x2e0 mm/kasan/generic.c:189
>>>>   instrument_atomic_read include/linux/instrumented.h:71 [inline]
>>>>   atomic_read include/linux/atomic/atomic-instrumented.h:27 [inline]
>>>>   buffer_busy fs/buffer.c:2856 [inline]
>>>>   drop_buffers+0x61/0x2f0 fs/buffer.c:2868
>>>>   try_to_free_buffers+0x2b1/0x640 fs/buffer.c:2898
>>>> [...]
>>>>
>>>> We use folio_has_private() to decide whether call filemap_release_folio(),
>>>> which may call try_to_free_buffers() then. folio_has_private() return true
>>>> for both PG_private and PG_private_2. We should only call try_to_free_buffers()
>>>> for case PG_private. So we should recheck PG_private in try_to_free_buffers().
>>>>
>>>> Reported-by: syzbot+fbdb4ec578ebdcfb9ed2@syzkaller.appspotmail.com
>>>> Fixes: 266cf658efcf ("FS-Cache: Recruit a page flags for cache management")
>>> but this can only happen for a filesystem which uses both bufferheads
>>> and PG_private_2.  afaik there aren't any of those in the tree.  so
>>> this bug can't actually happen.
>>>
>>> if you have your own filesystem that does, you need to submit it.
>> This null-ptr-deref is found by syzbot, not by my own filesystem. I review the related code and
>> found no other possible cause. There are lock protection all the place calling try_to_free_buffers().
>> So I only thought of this one possibility. I'm also trying to reproduce the problem but haven't
>> been successful.
>>
>> If this can't actually happen, maybe I'm missing something when review the code. I'll keep trying
>> to see if I can reproduce the problem.
> perhaps you could include more information, like the rest of the call
> stack so we can see what filesystem is involved?
This is the original link about the bug:
https://groups.google.com/g/syzkaller-bugs/c/sqeWJ62OEsc/m/kr6FRxXqBAAJ
>
> .
>