All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] fs: udf: super.c: Fix a use-after-free issue in udf_finalize_lvid
@ 2023-10-30 20:24 Shreeya Patel
  2023-10-31 11:37 ` Jan Kara
  0 siblings, 1 reply; 6+ messages in thread
From: Shreeya Patel @ 2023-10-30 20:24 UTC (permalink / raw)
  To: jack
  Cc: linux-kernel, kernel, groeck, zsm, Shreeya Patel,
	syzbot+82df44ede2faca24c729

Add some error handling cases in udf_sb_lvidiu() and redefine
the descCRCLength in order to avoid use-after-free issue in
udf_finalize_lvid.

Following use-after-free issue was reported by syzbot :-

https://syzkaller.appspot.com/bug?extid=46073c22edd7f242c028

BUG: KASAN: use-after-free in crc_itu_t+0x97/0xc8 lib/crc-itu-t.c:60
Read of size 1 at addr ffff88816fba0000 by task syz-executor.0/32133

Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 09/06/2023
Call Trace:
<TASK>
__dump_stack lib/dump_stack.c:88 [inline]
dump_stack_lvl+0x1e3/0x2cb lib/dump_stack.c:106
print_address_description mm/kasan/report.c:284 [inline]
print_report+0x13c/0x462 mm/kasan/report.c:395
kasan_report+0xa9/0xd5 mm/kasan/report.c:495
crc_itu_t+0x97/0xc8 lib/crc-itu-t.c:60
udf_finalize_lvid+0x111/0x23b fs/udf/super.c:2022
udf_sync_fs+0xba/0x123 fs/udf/super.c:2378
sync_filesystem+0xe8/0x216 fs/sync.c:56
generic_shutdown_super+0x6b/0x334 fs/super.c:474
kill_block_super+0x79/0xd6 fs/super.c:1459
deactivate_locked_super+0xa0/0x101 fs/super.c:332
cleanup_mnt+0x2de/0x361 fs/namespace.c:1192
task_work_run+0x22b/0x2d4 kernel/task_work.c:179
resume_user_mode_work include/linux/resume_user_mode.h:49 [inline]
exit_to_user_mode_loop+0xc4/0xd3 kernel/entry/common.c:171
exit_to_user_mode_prepare+0xb4/0x115 kernel/entry/common.c:204
__syscall_exit_to_user_mode_work kernel/entry/common.c:286 [inline]
syscall_exit_to_user_mode+0xae/0x278 kernel/entry/common.c:297
do_syscall_64+0x5d/0x93 arch/x86/entry/common.c:99
entry_SYSCALL_64_after_hwframe+0x63/0xcd
RIP: 0033:0x7e8195fb6e17

Fixes: ebbd5e99f60a ("udf: factor out LVID finalization for reuse")
Reported-by: syzbot+82df44ede2faca24c729@syzkaller.appspotmail.com
Signed-off-by: Shreeya Patel <shreeya.patel@collabora.com>
---
 fs/udf/super.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/fs/udf/super.c b/fs/udf/super.c
index 928a04d9d9e0..ca8f10eaa748 100644
--- a/fs/udf/super.c
+++ b/fs/udf/super.c
@@ -114,6 +114,10 @@ struct logicalVolIntegrityDescImpUse *udf_sb_lvidiu(struct super_block *sb)
 	partnum = le32_to_cpu(lvid->numOfPartitions);
 	/* The offset is to skip freeSpaceTable and sizeTable arrays */
 	offset = partnum * 2 * sizeof(uint32_t);
+	if (sb->s_blocksize < sizeof(*lvid) || (sb->s_blocksize - sizeof(*lvid)) <
+	    (offset + sizeof(struct logicalVolIntegrityDescImpUse)))
+		return NULL;
+
 	return (struct logicalVolIntegrityDescImpUse *)
 					(((uint8_t *)(lvid + 1)) + offset);
 }
@@ -2337,6 +2341,8 @@ static int udf_sync_fs(struct super_block *sb, int wait)
 		struct logicalVolIntegrityDesc *lvid;
 
 		lvid = (struct logicalVolIntegrityDesc *)bh->b_data;
+		if ((le16_to_cpu(lvid->descTag.descCRCLength) + sizeof(struct tag)) > sb->s_blocksize)
+			lvid->descTag.descCRCLength = cpu_to_le16(sb->s_blocksize - sizeof(struct tag));
 		udf_finalize_lvid(lvid);
 
 		/*
-- 
2.39.2


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH] fs: udf: super.c: Fix a use-after-free issue in udf_finalize_lvid
  2023-10-30 20:24 [PATCH] fs: udf: super.c: Fix a use-after-free issue in udf_finalize_lvid Shreeya Patel
@ 2023-10-31 11:37 ` Jan Kara
  2023-11-02 10:04   ` Shreeya Patel
  0 siblings, 1 reply; 6+ messages in thread
From: Jan Kara @ 2023-10-31 11:37 UTC (permalink / raw)
  To: Shreeya Patel
  Cc: jack, linux-kernel, kernel, groeck, zsm, syzbot+82df44ede2faca24c729

On Tue 31-10-23 01:54:18, Shreeya Patel wrote:
> Add some error handling cases in udf_sb_lvidiu() and redefine
> the descCRCLength in order to avoid use-after-free issue in
> udf_finalize_lvid.
> 
> Following use-after-free issue was reported by syzbot :-
> 
> https://syzkaller.appspot.com/bug?extid=46073c22edd7f242c028
> 
> BUG: KASAN: use-after-free in crc_itu_t+0x97/0xc8 lib/crc-itu-t.c:60
> Read of size 1 at addr ffff88816fba0000 by task syz-executor.0/32133
> 
> Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 09/06/2023
> Call Trace:
> <TASK>
> __dump_stack lib/dump_stack.c:88 [inline]
> dump_stack_lvl+0x1e3/0x2cb lib/dump_stack.c:106
> print_address_description mm/kasan/report.c:284 [inline]
> print_report+0x13c/0x462 mm/kasan/report.c:395
> kasan_report+0xa9/0xd5 mm/kasan/report.c:495
> crc_itu_t+0x97/0xc8 lib/crc-itu-t.c:60
> udf_finalize_lvid+0x111/0x23b fs/udf/super.c:2022
> udf_sync_fs+0xba/0x123 fs/udf/super.c:2378
> sync_filesystem+0xe8/0x216 fs/sync.c:56
> generic_shutdown_super+0x6b/0x334 fs/super.c:474
> kill_block_super+0x79/0xd6 fs/super.c:1459
> deactivate_locked_super+0xa0/0x101 fs/super.c:332
> cleanup_mnt+0x2de/0x361 fs/namespace.c:1192
> task_work_run+0x22b/0x2d4 kernel/task_work.c:179
> resume_user_mode_work include/linux/resume_user_mode.h:49 [inline]
> exit_to_user_mode_loop+0xc4/0xd3 kernel/entry/common.c:171
> exit_to_user_mode_prepare+0xb4/0x115 kernel/entry/common.c:204
> __syscall_exit_to_user_mode_work kernel/entry/common.c:286 [inline]
> syscall_exit_to_user_mode+0xae/0x278 kernel/entry/common.c:297
> do_syscall_64+0x5d/0x93 arch/x86/entry/common.c:99
> entry_SYSCALL_64_after_hwframe+0x63/0xcd
> RIP: 0033:0x7e8195fb6e17
> 
> Fixes: ebbd5e99f60a ("udf: factor out LVID finalization for reuse")
> Reported-by: syzbot+82df44ede2faca24c729@syzkaller.appspotmail.com
> Signed-off-by: Shreeya Patel <shreeya.patel@collabora.com>

Thanks for the patch but not every syzbot report is actually a bug. In this
case you can notice that udf_load_logicalvolint() is actually checking
validity of the Logical Volume Integrity Descriptor. The fact that later
udf_sb_lvidiu() call overflows the buffer size is caused by the fact that
syzbot overwrites the UDF filesystem while it is mounted and so the values
we checked are not the same as the value we later use. That is not a
problem we try to protect against (it is equivalent to corrupting memory). 
I'm working on patches to so that syzbot can reasonably easily avoid
creating such invalid scenarios but so far they did not land. So I'm sorry
but I will not apply your fix.

								Honza


> ---
>  fs/udf/super.c | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/fs/udf/super.c b/fs/udf/super.c
> index 928a04d9d9e0..ca8f10eaa748 100644
> --- a/fs/udf/super.c
> +++ b/fs/udf/super.c
> @@ -114,6 +114,10 @@ struct logicalVolIntegrityDescImpUse *udf_sb_lvidiu(struct super_block *sb)
>  	partnum = le32_to_cpu(lvid->numOfPartitions);
>  	/* The offset is to skip freeSpaceTable and sizeTable arrays */
>  	offset = partnum * 2 * sizeof(uint32_t);
> +	if (sb->s_blocksize < sizeof(*lvid) || (sb->s_blocksize - sizeof(*lvid)) <
> +	    (offset + sizeof(struct logicalVolIntegrityDescImpUse)))
> +		return NULL;
> +
>  	return (struct logicalVolIntegrityDescImpUse *)
>  					(((uint8_t *)(lvid + 1)) + offset);
>  }
> @@ -2337,6 +2341,8 @@ static int udf_sync_fs(struct super_block *sb, int wait)
>  		struct logicalVolIntegrityDesc *lvid;
>  
>  		lvid = (struct logicalVolIntegrityDesc *)bh->b_data;
> +		if ((le16_to_cpu(lvid->descTag.descCRCLength) + sizeof(struct tag)) > sb->s_blocksize)
> +			lvid->descTag.descCRCLength = cpu_to_le16(sb->s_blocksize - sizeof(struct tag));
>  		udf_finalize_lvid(lvid);
>  
>  		/*
> -- 
> 2.39.2
> 
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] fs: udf: super.c: Fix a use-after-free issue in udf_finalize_lvid
  2023-10-31 11:37 ` Jan Kara
@ 2023-11-02 10:04   ` Shreeya Patel
  2023-11-02 11:05     ` Jan Kara
  0 siblings, 1 reply; 6+ messages in thread
From: Shreeya Patel @ 2023-11-02 10:04 UTC (permalink / raw)
  To: Jan Kara
  Cc: jack, linux-kernel, kernel, groeck, zsm, syzbot+82df44ede2faca24c729



On 31/10/23 17:07, Jan Kara wrote:
> On Tue 31-10-23 01:54:18, Shreeya Patel wrote:
>> Add some error handling cases in udf_sb_lvidiu() and redefine
>> the descCRCLength in order to avoid use-after-free issue in
>> udf_finalize_lvid.
>>
>> Following use-after-free issue was reported by syzbot :-
>>
>> https://syzkaller.appspot.com/bug?extid=46073c22edd7f242c028
>>
>> BUG: KASAN: use-after-free in crc_itu_t+0x97/0xc8 lib/crc-itu-t.c:60
>> Read of size 1 at addr ffff88816fba0000 by task syz-executor.0/32133
>>
>> Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 09/06/2023
>> Call Trace:
>> <TASK>
>> __dump_stack lib/dump_stack.c:88 [inline]
>> dump_stack_lvl+0x1e3/0x2cb lib/dump_stack.c:106
>> print_address_description mm/kasan/report.c:284 [inline]
>> print_report+0x13c/0x462 mm/kasan/report.c:395
>> kasan_report+0xa9/0xd5 mm/kasan/report.c:495
>> crc_itu_t+0x97/0xc8 lib/crc-itu-t.c:60
>> udf_finalize_lvid+0x111/0x23b fs/udf/super.c:2022
>> udf_sync_fs+0xba/0x123 fs/udf/super.c:2378
>> sync_filesystem+0xe8/0x216 fs/sync.c:56
>> generic_shutdown_super+0x6b/0x334 fs/super.c:474
>> kill_block_super+0x79/0xd6 fs/super.c:1459
>> deactivate_locked_super+0xa0/0x101 fs/super.c:332
>> cleanup_mnt+0x2de/0x361 fs/namespace.c:1192
>> task_work_run+0x22b/0x2d4 kernel/task_work.c:179
>> resume_user_mode_work include/linux/resume_user_mode.h:49 [inline]
>> exit_to_user_mode_loop+0xc4/0xd3 kernel/entry/common.c:171
>> exit_to_user_mode_prepare+0xb4/0x115 kernel/entry/common.c:204
>> __syscall_exit_to_user_mode_work kernel/entry/common.c:286 [inline]
>> syscall_exit_to_user_mode+0xae/0x278 kernel/entry/common.c:297
>> do_syscall_64+0x5d/0x93 arch/x86/entry/common.c:99
>> entry_SYSCALL_64_after_hwframe+0x63/0xcd
>> RIP: 0033:0x7e8195fb6e17
>>
>> Fixes: ebbd5e99f60a ("udf: factor out LVID finalization for reuse")
>> Reported-by: syzbot+82df44ede2faca24c729@syzkaller.appspotmail.com
>> Signed-off-by: Shreeya Patel <shreeya.patel@collabora.com>
> Thanks for the patch but not every syzbot report is actually a bug. In this
> case you can notice that udf_load_logicalvolint() is actually checking
> validity of the Logical Volume Integrity Descriptor. The fact that later
> udf_sb_lvidiu() call overflows the buffer size is caused by the fact that
> syzbot overwrites the UDF filesystem while it is mounted and so the values
> we checked are not the same as the value we later use. That is not a
> problem we try to protect against (it is equivalent to corrupting memory).
> I'm working on patches to so that syzbot can reasonably easily avoid
> creating such invalid scenarios but so far they did not land. So I'm sorry
> but I will not apply your fix.

Hi Jan,

Thanks for the information and it definitely makes sense to not let syzbot
create such invalid scenarios. Maybe we can add some kind of filtering 
in syzbot
for these kind of issues in future but I wonder how to even identify 
these reports from syzbot
which is purposely trying to do some memory corruption. It seems hard to 
identify them
without understanding what the reproducer is doing.
Maybe this is a question for the syzbot team.



Thanks
Shreeya Patel
> 								Honza
>
>
>> ---
>>   fs/udf/super.c | 6 ++++++
>>   1 file changed, 6 insertions(+)
>>
>> diff --git a/fs/udf/super.c b/fs/udf/super.c
>> index 928a04d9d9e0..ca8f10eaa748 100644
>> --- a/fs/udf/super.c
>> +++ b/fs/udf/super.c
>> @@ -114,6 +114,10 @@ struct logicalVolIntegrityDescImpUse *udf_sb_lvidiu(struct super_block *sb)
>>   	partnum = le32_to_cpu(lvid->numOfPartitions);
>>   	/* The offset is to skip freeSpaceTable and sizeTable arrays */
>>   	offset = partnum * 2 * sizeof(uint32_t);
>> +	if (sb->s_blocksize < sizeof(*lvid) || (sb->s_blocksize - sizeof(*lvid)) <
>> +	    (offset + sizeof(struct logicalVolIntegrityDescImpUse)))
>> +		return NULL;
>> +
>>   	return (struct logicalVolIntegrityDescImpUse *)
>>   					(((uint8_t *)(lvid + 1)) + offset);
>>   }
>> @@ -2337,6 +2341,8 @@ static int udf_sync_fs(struct super_block *sb, int wait)
>>   		struct logicalVolIntegrityDesc *lvid;
>>   
>>   		lvid = (struct logicalVolIntegrityDesc *)bh->b_data;
>> +		if ((le16_to_cpu(lvid->descTag.descCRCLength) + sizeof(struct tag)) > sb->s_blocksize)
>> +			lvid->descTag.descCRCLength = cpu_to_le16(sb->s_blocksize - sizeof(struct tag));
>>   		udf_finalize_lvid(lvid);
>>   
>>   		/*
>> -- 
>> 2.39.2
>>


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] fs: udf: super.c: Fix a use-after-free issue in udf_finalize_lvid
  2023-11-02 10:04   ` Shreeya Patel
@ 2023-11-02 11:05     ` Jan Kara
  2023-11-02 11:07       ` Jan Kara
  0 siblings, 1 reply; 6+ messages in thread
From: Jan Kara @ 2023-11-02 11:05 UTC (permalink / raw)
  To: Shreeya Patel
  Cc: Jan Kara, jack, linux-kernel, kernel, groeck, zsm,
	syzbot+82df44ede2faca24c729

On Thu 02-11-23 15:34:52, Shreeya Patel wrote:
> On 31/10/23 17:07, Jan Kara wrote:
> > On Tue 31-10-23 01:54:18, Shreeya Patel wrote:
> > > Add some error handling cases in udf_sb_lvidiu() and redefine
> > > the descCRCLength in order to avoid use-after-free issue in
> > > udf_finalize_lvid.
> > > 
> > > Following use-after-free issue was reported by syzbot :-
> > > 
> > > https://syzkaller.appspot.com/bug?extid=46073c22edd7f242c028
> > > 
> > > BUG: KASAN: use-after-free in crc_itu_t+0x97/0xc8 lib/crc-itu-t.c:60
> > > Read of size 1 at addr ffff88816fba0000 by task syz-executor.0/32133
> > > 
> > > Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 09/06/2023
> > > Call Trace:
> > > <TASK>
> > > __dump_stack lib/dump_stack.c:88 [inline]
> > > dump_stack_lvl+0x1e3/0x2cb lib/dump_stack.c:106
> > > print_address_description mm/kasan/report.c:284 [inline]
> > > print_report+0x13c/0x462 mm/kasan/report.c:395
> > > kasan_report+0xa9/0xd5 mm/kasan/report.c:495
> > > crc_itu_t+0x97/0xc8 lib/crc-itu-t.c:60
> > > udf_finalize_lvid+0x111/0x23b fs/udf/super.c:2022
> > > udf_sync_fs+0xba/0x123 fs/udf/super.c:2378
> > > sync_filesystem+0xe8/0x216 fs/sync.c:56
> > > generic_shutdown_super+0x6b/0x334 fs/super.c:474
> > > kill_block_super+0x79/0xd6 fs/super.c:1459
> > > deactivate_locked_super+0xa0/0x101 fs/super.c:332
> > > cleanup_mnt+0x2de/0x361 fs/namespace.c:1192
> > > task_work_run+0x22b/0x2d4 kernel/task_work.c:179
> > > resume_user_mode_work include/linux/resume_user_mode.h:49 [inline]
> > > exit_to_user_mode_loop+0xc4/0xd3 kernel/entry/common.c:171
> > > exit_to_user_mode_prepare+0xb4/0x115 kernel/entry/common.c:204
> > > __syscall_exit_to_user_mode_work kernel/entry/common.c:286 [inline]
> > > syscall_exit_to_user_mode+0xae/0x278 kernel/entry/common.c:297
> > > do_syscall_64+0x5d/0x93 arch/x86/entry/common.c:99
> > > entry_SYSCALL_64_after_hwframe+0x63/0xcd
> > > RIP: 0033:0x7e8195fb6e17
> > > 
> > > Fixes: ebbd5e99f60a ("udf: factor out LVID finalization for reuse")
> > > Reported-by: syzbot+82df44ede2faca24c729@syzkaller.appspotmail.com
> > > Signed-off-by: Shreeya Patel <shreeya.patel@collabora.com>
> > Thanks for the patch but not every syzbot report is actually a bug. In this
> > case you can notice that udf_load_logicalvolint() is actually checking
> > validity of the Logical Volume Integrity Descriptor. The fact that later
> > udf_sb_lvidiu() call overflows the buffer size is caused by the fact that
> > syzbot overwrites the UDF filesystem while it is mounted and so the values
> > we checked are not the same as the value we later use. That is not a
> > problem we try to protect against (it is equivalent to corrupting memory).
> > I'm working on patches to so that syzbot can reasonably easily avoid
> > creating such invalid scenarios but so far they did not land. So I'm sorry
> > but I will not apply your fix.
> 
> Thanks for the information and it definitely makes sense to not let
> syzbot create such invalid scenarios. Maybe we can add some kind of
> filtering in syzbot for these kind of issues in future but I wonder how
> to even identify these reports from syzbot which is purposely trying to
> do some memory corruption. It seems hard to identify them without
> understanding what the reproducer is doing.  Maybe this is a question for
> the syzbot team.

I have discussed this with the syzbot team and as you noticed the problem
is it is very hard to detect the corruption scenario in an automated way.
What we plan to do (next round of patches submitted yesterday [1])

[1]

-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] fs: udf: super.c: Fix a use-after-free issue in udf_finalize_lvid
  2023-11-02 11:05     ` Jan Kara
@ 2023-11-02 11:07       ` Jan Kara
  2023-11-03  8:45         ` Shreeya Patel
  0 siblings, 1 reply; 6+ messages in thread
From: Jan Kara @ 2023-11-02 11:07 UTC (permalink / raw)
  To: Shreeya Patel
  Cc: Jan Kara, jack, linux-kernel, kernel, groeck, zsm,
	syzbot+82df44ede2faca24c729

On Thu 02-11-23 12:05:10, Jan Kara wrote:
> On Thu 02-11-23 15:34:52, Shreeya Patel wrote:
> > On 31/10/23 17:07, Jan Kara wrote:
> > > On Tue 31-10-23 01:54:18, Shreeya Patel wrote:
> > > > Add some error handling cases in udf_sb_lvidiu() and redefine
> > > > the descCRCLength in order to avoid use-after-free issue in
> > > > udf_finalize_lvid.
> > > > 
> > > > Following use-after-free issue was reported by syzbot :-
> > > > 
> > > > https://syzkaller.appspot.com/bug?extid=46073c22edd7f242c028
> > > > 
> > > > BUG: KASAN: use-after-free in crc_itu_t+0x97/0xc8 lib/crc-itu-t.c:60
> > > > Read of size 1 at addr ffff88816fba0000 by task syz-executor.0/32133
> > > > 
> > > > Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 09/06/2023
> > > > Call Trace:
> > > > <TASK>
> > > > __dump_stack lib/dump_stack.c:88 [inline]
> > > > dump_stack_lvl+0x1e3/0x2cb lib/dump_stack.c:106
> > > > print_address_description mm/kasan/report.c:284 [inline]
> > > > print_report+0x13c/0x462 mm/kasan/report.c:395
> > > > kasan_report+0xa9/0xd5 mm/kasan/report.c:495
> > > > crc_itu_t+0x97/0xc8 lib/crc-itu-t.c:60
> > > > udf_finalize_lvid+0x111/0x23b fs/udf/super.c:2022
> > > > udf_sync_fs+0xba/0x123 fs/udf/super.c:2378
> > > > sync_filesystem+0xe8/0x216 fs/sync.c:56
> > > > generic_shutdown_super+0x6b/0x334 fs/super.c:474
> > > > kill_block_super+0x79/0xd6 fs/super.c:1459
> > > > deactivate_locked_super+0xa0/0x101 fs/super.c:332
> > > > cleanup_mnt+0x2de/0x361 fs/namespace.c:1192
> > > > task_work_run+0x22b/0x2d4 kernel/task_work.c:179
> > > > resume_user_mode_work include/linux/resume_user_mode.h:49 [inline]
> > > > exit_to_user_mode_loop+0xc4/0xd3 kernel/entry/common.c:171
> > > > exit_to_user_mode_prepare+0xb4/0x115 kernel/entry/common.c:204
> > > > __syscall_exit_to_user_mode_work kernel/entry/common.c:286 [inline]
> > > > syscall_exit_to_user_mode+0xae/0x278 kernel/entry/common.c:297
> > > > do_syscall_64+0x5d/0x93 arch/x86/entry/common.c:99
> > > > entry_SYSCALL_64_after_hwframe+0x63/0xcd
> > > > RIP: 0033:0x7e8195fb6e17
> > > > 
> > > > Fixes: ebbd5e99f60a ("udf: factor out LVID finalization for reuse")
> > > > Reported-by: syzbot+82df44ede2faca24c729@syzkaller.appspotmail.com
> > > > Signed-off-by: Shreeya Patel <shreeya.patel@collabora.com>
> > > Thanks for the patch but not every syzbot report is actually a bug. In this
> > > case you can notice that udf_load_logicalvolint() is actually checking
> > > validity of the Logical Volume Integrity Descriptor. The fact that later
> > > udf_sb_lvidiu() call overflows the buffer size is caused by the fact that
> > > syzbot overwrites the UDF filesystem while it is mounted and so the values
> > > we checked are not the same as the value we later use. That is not a
> > > problem we try to protect against (it is equivalent to corrupting memory).
> > > I'm working on patches to so that syzbot can reasonably easily avoid
> > > creating such invalid scenarios but so far they did not land. So I'm sorry
> > > but I will not apply your fix.
> > 
> > Thanks for the information and it definitely makes sense to not let
> > syzbot create such invalid scenarios. Maybe we can add some kind of
> > filtering in syzbot for these kind of issues in future but I wonder how
> > to even identify these reports from syzbot which is purposely trying to
> > do some memory corruption. It seems hard to identify them without
> > understanding what the reproducer is doing.  Maybe this is a question for
> > the syzbot team.
 
Hit send too early ;)

I have discussed this with the syzbot team and as you noticed the problem
is it is very hard to detect the corruption scenario in an automated way.
What we plan to do (next round of patches submitted yesterday [1]) is that
we will not allow processes to open devices for writing when they are
mounted. This will effectively not allow syzbot to corrupt buffer cache of
a mounted filesystem and so should address these issues.

								Honza
 
[1] https://lore.kernel.org/all/20231101173542.23597-1-jack@suse.cz

-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] fs: udf: super.c: Fix a use-after-free issue in udf_finalize_lvid
  2023-11-02 11:07       ` Jan Kara
@ 2023-11-03  8:45         ` Shreeya Patel
  0 siblings, 0 replies; 6+ messages in thread
From: Shreeya Patel @ 2023-11-03  8:45 UTC (permalink / raw)
  To: Jan Kara
  Cc: jack, linux-kernel, kernel, groeck, zsm, syzbot+82df44ede2faca24c729



On 02/11/23 16:37, Jan Kara wrote:
> On Thu 02-11-23 12:05:10, Jan Kara wrote:
>> On Thu 02-11-23 15:34:52, Shreeya Patel wrote:
>>> On 31/10/23 17:07, Jan Kara wrote:
>>>> On Tue 31-10-23 01:54:18, Shreeya Patel wrote:
>>>>> Add some error handling cases in udf_sb_lvidiu() and redefine
>>>>> the descCRCLength in order to avoid use-after-free issue in
>>>>> udf_finalize_lvid.
>>>>>
>>>>> Following use-after-free issue was reported by syzbot :-
>>>>>
>>>>> https://syzkaller.appspot.com/bug?extid=46073c22edd7f242c028
>>>>>
>>>>> BUG: KASAN: use-after-free in crc_itu_t+0x97/0xc8 lib/crc-itu-t.c:60
>>>>> Read of size 1 at addr ffff88816fba0000 by task syz-executor.0/32133
>>>>>
>>>>> Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 09/06/2023
>>>>> Call Trace:
>>>>> <TASK>
>>>>> __dump_stack lib/dump_stack.c:88 [inline]
>>>>> dump_stack_lvl+0x1e3/0x2cb lib/dump_stack.c:106
>>>>> print_address_description mm/kasan/report.c:284 [inline]
>>>>> print_report+0x13c/0x462 mm/kasan/report.c:395
>>>>> kasan_report+0xa9/0xd5 mm/kasan/report.c:495
>>>>> crc_itu_t+0x97/0xc8 lib/crc-itu-t.c:60
>>>>> udf_finalize_lvid+0x111/0x23b fs/udf/super.c:2022
>>>>> udf_sync_fs+0xba/0x123 fs/udf/super.c:2378
>>>>> sync_filesystem+0xe8/0x216 fs/sync.c:56
>>>>> generic_shutdown_super+0x6b/0x334 fs/super.c:474
>>>>> kill_block_super+0x79/0xd6 fs/super.c:1459
>>>>> deactivate_locked_super+0xa0/0x101 fs/super.c:332
>>>>> cleanup_mnt+0x2de/0x361 fs/namespace.c:1192
>>>>> task_work_run+0x22b/0x2d4 kernel/task_work.c:179
>>>>> resume_user_mode_work include/linux/resume_user_mode.h:49 [inline]
>>>>> exit_to_user_mode_loop+0xc4/0xd3 kernel/entry/common.c:171
>>>>> exit_to_user_mode_prepare+0xb4/0x115 kernel/entry/common.c:204
>>>>> __syscall_exit_to_user_mode_work kernel/entry/common.c:286 [inline]
>>>>> syscall_exit_to_user_mode+0xae/0x278 kernel/entry/common.c:297
>>>>> do_syscall_64+0x5d/0x93 arch/x86/entry/common.c:99
>>>>> entry_SYSCALL_64_after_hwframe+0x63/0xcd
>>>>> RIP: 0033:0x7e8195fb6e17
>>>>>
>>>>> Fixes: ebbd5e99f60a ("udf: factor out LVID finalization for reuse")
>>>>> Reported-by: syzbot+82df44ede2faca24c729@syzkaller.appspotmail.com
>>>>> Signed-off-by: Shreeya Patel <shreeya.patel@collabora.com>
>>>> Thanks for the patch but not every syzbot report is actually a bug. In this
>>>> case you can notice that udf_load_logicalvolint() is actually checking
>>>> validity of the Logical Volume Integrity Descriptor. The fact that later
>>>> udf_sb_lvidiu() call overflows the buffer size is caused by the fact that
>>>> syzbot overwrites the UDF filesystem while it is mounted and so the values
>>>> we checked are not the same as the value we later use. That is not a
>>>> problem we try to protect against (it is equivalent to corrupting memory).
>>>> I'm working on patches to so that syzbot can reasonably easily avoid
>>>> creating such invalid scenarios but so far they did not land. So I'm sorry
>>>> but I will not apply your fix.
>>> Thanks for the information and it definitely makes sense to not let
>>> syzbot create such invalid scenarios. Maybe we can add some kind of
>>> filtering in syzbot for these kind of issues in future but I wonder how
>>> to even identify these reports from syzbot which is purposely trying to
>>> do some memory corruption. It seems hard to identify them without
>>> understanding what the reproducer is doing.  Maybe this is a question for
>>> the syzbot team.
>   
> Hit send too early ;)
>
> I have discussed this with the syzbot team and as you noticed the problem
> is it is very hard to detect the corruption scenario in an automated way.
> What we plan to do (next round of patches submitted yesterday [1]) is that
> we will not allow processes to open devices for writing when they are
> mounted. This will effectively not allow syzbot to corrupt buffer cache of
> a mounted filesystem and so should address these issues.

Thanks for this work, it will save us some time as well :)


Regards,
Shreeya Patel

> 								Honza
>   
> [1] https://lore.kernel.org/all/20231101173542.23597-1-jack@suse.cz
>


^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2023-11-03  8:45 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-10-30 20:24 [PATCH] fs: udf: super.c: Fix a use-after-free issue in udf_finalize_lvid Shreeya Patel
2023-10-31 11:37 ` Jan Kara
2023-11-02 10:04   ` Shreeya Patel
2023-11-02 11:05     ` Jan Kara
2023-11-02 11:07       ` Jan Kara
2023-11-03  8:45         ` Shreeya Patel

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.