[syzbot] [ocfs2?] kernel BUG in ocfs2_truncate_inline

[syzbot] [ocfs2?] kernel BUG in ocfs2_truncate_inline

[syzbot]

Hello,

syzbot found the following issue on:

HEAD commit:    360c1f1f24c6 Merge tag 'block-6.12-20241004' of git://git...
git tree:       upstream
console+strace: https://syzkaller.appspot.com/x/log.txt?x=1208479f980000
kernel config:  https://syzkaller.appspot.com/x/.config?x=f8af0b3195caed62
dashboard link: https://syzkaller.appspot.com/bug?extid=81092778aac03460d6b7
compiler:       Debian clang version 15.0.6, GNU ld (GNU Binutils for Debian) 2.40
syz repro:      https://syzkaller.appspot.com/x/repro.syz?x=17f6d380580000
C reproducer:   https://syzkaller.appspot.com/x/repro.c?x=1608479f980000

Downloadable assets:
disk image: https://storage.googleapis.com/syzbot-assets/518c06e21f9f/disk-360c1f1f.raw.xz
vmlinux: https://storage.googleapis.com/syzbot-assets/2a385fca5995/vmlinux-360c1f1f.xz
kernel image: https://storage.googleapis.com/syzbot-assets/510ff8226499/bzImage-360c1f1f.xz
mounted in repro: https://storage.googleapis.com/syzbot-assets/fc8df043f98c/mount_0.gz

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

bisection log:  https://syzkaller.appspot.com/x/bisect.txt?x=135dcd27980000
final oops:     https://syzkaller.appspot.com/x/report.txt?x=10ddcd27980000
console output: https://syzkaller.appspot.com/x/log.txt?x=175dcd27980000

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

ocfs2: Mounting device (7,0) on (node local, slot 0) with ordered data mode.
------------[ cut here ]------------
kernel BUG at fs/ocfs2/alloc.c:7402!
Oops: invalid opcode: 0000 [#1] PREEMPT SMP KASAN PTI
CPU: 1 UID: 0 PID: 5222 Comm: syz-executor553 Not tainted 6.12.0-rc1-syzkaller-00165-g360c1f1f24c6 #0
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 09/13/2024
RIP: 0010:ocfs2_truncate_inline+0x93d/0x940 fs/ocfs2/alloc.c:7402
Code: 38 c1 0f 8c bf fe ff ff 48 89 d7 48 89 54 24 08 e8 d8 d6 7a fe 48 8b 54 24 08 e9 a8 fe ff ff e8 f9 37 3c 08 e8 24 48 14 fe 90 <0f> 0b 90 90 90 90 90 90 90 90 90 90 90 90 90 90 90 90 90 f3 0f 1e
RSP: 0018:ffffc900038ef200 EFLAGS: 00010293
RAX: ffffffff83808e6c RBX: 00000000fffffffd RCX: ffff88802ae61e00
RDX: 0000000000000000 RSI: 0000000000000007 RDI: 0000000000000004
RBP: ffffc900038ef310 R08: ffffffff83808708 R09: 1ffffffff20377cd
R10: dffffc0000000000 R11: fffffbfff20377ce R12: 0000000000000007
R13: ffff888074ba5e48 R14: dffffc0000000000 R15: 0000000000000007
FS:  0000555583ee2380(0000) GS:ffff8880b8700000(0000) knlGS:0000000000000000
CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 00007f7782abb100 CR3: 0000000029b44000 CR4: 00000000003526f0
DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
Call Trace:
 <TASK>
 ocfs2_remove_inode_range+0x603/0x2690 fs/ocfs2/file.c:1787
 __ocfs2_change_file_space+0x8e4/0xfd0 fs/ocfs2/file.c:2017
 ocfs2_fallocate+0x2e4/0x350 fs/ocfs2/file.c:2127
 vfs_fallocate+0x569/0x6e0 fs/open.c:333
 do_vfs_ioctl+0x2583/0x2e40 fs/ioctl.c:886
 __do_sys_ioctl fs/ioctl.c:905 [inline]
 __se_sys_ioctl+0x81/0x170 fs/ioctl.c:893
 do_syscall_x64 arch/x86/entry/common.c:52 [inline]
 do_syscall_64+0xf3/0x230 arch/x86/entry/common.c:83
 entry_SYSCALL_64_after_hwframe+0x77/0x7f
RIP: 0033:0x7f7782a3edd9
Code: 28 00 00 00 75 05 48 83 c4 28 c3 e8 f1 17 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 b8 ff ff ff f7 d8 64 89 01 48
RSP: 002b:00007ffcbc179258 EFLAGS: 00000246 ORIG_RAX: 0000000000000010
RAX: ffffffffffffffda RBX: 0031656c69662f2e RCX: 00007f7782a3edd9
RDX: 00000000200000c0 RSI: 0000000040305829 RDI: 0000000000000005
RBP: 00007f7782ab75f0 R08: 0000555583ee34c0 R09: 0000555583ee34c0
R10: 0000555583ee34c0 R11: 0000000000000246 R12: 00007ffcbc179280
R13: 00007ffcbc1794a8 R14: 431bde82d7b634db R15: 00007f7782a8803b
 </TASK>
Modules linked in:
---[ end trace 0000000000000000 ]---
RIP: 0010:ocfs2_truncate_inline+0x93d/0x940 fs/ocfs2/alloc.c:7402
Code: 38 c1 0f 8c bf fe ff ff 48 89 d7 48 89 54 24 08 e8 d8 d6 7a fe 48 8b 54 24 08 e9 a8 fe ff ff e8 f9 37 3c 08 e8 24 48 14 fe 90 <0f> 0b 90 90 90 90 90 90 90 90 90 90 90 90 90 90 90 90 90 f3 0f 1e
RSP: 0018:ffffc900038ef200 EFLAGS: 00010293
RAX: ffffffff83808e6c RBX: 00000000fffffffd RCX: ffff88802ae61e00
RDX: 0000000000000000 RSI: 0000000000000007 RDI: 0000000000000004
RBP: ffffc900038ef310 R08: ffffffff83808708 R09: 1ffffffff20377cd
R10: dffffc0000000000 R11: fffffbfff20377ce R12: 0000000000000007
R13: ffff888074ba5e48 R14: dffffc0000000000 R15: 0000000000000007
FS:  0000555583ee2380(0000) GS:ffff8880b8700000(0000) knlGS:0000000000000000
CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 00007f7782abb100 CR3: 0000000029b44000 CR4: 00000000003526f0
DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400


---
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 syzkaller@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

If the report is already addressed, let syzbot know by replying with:
#syz fix: exact-commit-title

If you want syzbot to run the reproducer, reply with:
#syz test: git://repo/address.git branch-or-commit-hash
If you attach or paste a git patch, syzbot will apply it before testing.

If you want to overwrite report's subsystems, reply with:
#syz set subsystems: new-subsystem
(See the list of subsystem names on the web dashboard)

If the report is a duplicate of another one, reply with:
#syz dup: exact-subject-of-another-report

If you want to undo deduplication, reply with:
#syz undup

Re: [ocfs2?] kernel BUG in ocfs2_truncate_inline

[Edward Adam Davis]

offset or offset + len greater than UINT_MAX, if true, it will overflow in ocfs2_truncate_inline

#syz test

diff --git a/fs/ocfs2/file.c b/fs/ocfs2/file.c
index ad131a2fc58e..ed26ec8ac6b6 100644
--- a/fs/ocfs2/file.c
+++ b/fs/ocfs2/file.c
@@ -2117,6 +2117,9 @@ static long ocfs2_fallocate(struct file *file, int mode, loff_t offset,
 			return ret;
 	}
 
+	if (offset > UINT_MAX || offset + len > UINT_MAX)
+		return -EFBIG;
+
 	if (mode & FALLOC_FL_PUNCH_HOLE)
 		cmd = OCFS2_IOC_UNRESVSP64;
 

Re: [syzbot] [ocfs2?] kernel BUG in ocfs2_truncate_inline

[syzbot]

Hello,

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

Reported-by: syzbot+81092778aac03460d6b7@syzkaller.appspotmail.com
Tested-by: syzbot+81092778aac03460d6b7@syzkaller.appspotmail.com

Tested on:

commit:         75b607fa Merge tag 'sched_ext-for-6.12-rc2-fixes' of g..
git tree:       upstream
console output: https://syzkaller.appspot.com/x/log.txt?x=11060f07980000
kernel config:  https://syzkaller.appspot.com/x/.config?x=7a3fccdd0bb995
dashboard link: https://syzkaller.appspot.com/bug?extid=81092778aac03460d6b7
compiler:       Debian clang version 15.0.6, GNU ld (GNU Binutils for Debian) 2.40
patch:          https://syzkaller.appspot.com/x/patch.diff?x=142c5780580000

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

[PATCH] ocfs2: pass u64 to ocfs2_truncate_inline maybe overflow

[Edward Adam Davis]

Syzbot reported a kernel BUG in ocfs2_truncate_inline.
There are two reasons for this: first, the parameter value passed is greater
than UINT_MAX, second, the start and end parameters of ocfs2_truncate_inline
are "unsigned int".

So, we need to add a sanity check for offset and len in ocfs2_fallocate, if
they are greater than UINT_MAX return -EFBIG.

Reported-and-tested-by: syzbot+81092778aac03460d6b7@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=81092778aac03460d6b7
Signed-off-by: Edward Adam Davis <eadavis@qq.com>
---
 fs/ocfs2/file.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/fs/ocfs2/file.c b/fs/ocfs2/file.c
index ad131a2fc58e..ed26ec8ac6b6 100644
--- a/fs/ocfs2/file.c
+++ b/fs/ocfs2/file.c
@@ -2117,6 +2117,9 @@ static long ocfs2_fallocate(struct file *file, int mode, loff_t offset,
 			return ret;
 	}
 
+	if (offset > UINT_MAX || offset + len > UINT_MAX)
+		return -EFBIG;
+
 	if (mode & FALLOC_FL_PUNCH_HOLE)
 		cmd = OCFS2_IOC_UNRESVSP64;
 
-- 
2.43.0

Re: [PATCH] ocfs2: pass u64 to ocfs2_truncate_inline maybe overflow

[Joseph Qi]

On 10/9/24 11:05 PM, Edward Adam Davis wrote:
> Syzbot reported a kernel BUG in ocfs2_truncate_inline.
> There are two reasons for this: first, the parameter value passed is greater
> than UINT_MAX, second, the start and end parameters of ocfs2_truncate_inline
> are "unsigned int".
> 
> So, we need to add a sanity check for offset and len in ocfs2_fallocate, if
> they are greater than UINT_MAX return -EFBIG.

fallocate should accept loff_t (aka long long) offset and len.
I guess the reported bug is caused by a crafted image, which set
overflow offset and len in case of inline data (with flag
OCFS2_INLINE_DATA_FL set).
So IMO, the right place to add a sanity check is right before
ocfs2_truncate_inline() in ocfs2_remove_inode_range().

Thanks,
Joseph

> 
> Reported-and-tested-by: syzbot+81092778aac03460d6b7@syzkaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=81092778aac03460d6b7
> Signed-off-by: Edward Adam Davis <eadavis@qq.com>
> ---
>  fs/ocfs2/file.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/fs/ocfs2/file.c b/fs/ocfs2/file.c
> index ad131a2fc58e..ed26ec8ac6b6 100644
> --- a/fs/ocfs2/file.c
> +++ b/fs/ocfs2/file.c
> @@ -2117,6 +2117,9 @@ static long ocfs2_fallocate(struct file *file, int mode, loff_t offset,
>  			return ret;
>  	}
>  
> +	if (offset > UINT_MAX || offset + len > UINT_MAX)
> +		return -EFBIG;
> +
>  	if (mode & FALLOC_FL_PUNCH_HOLE)
>  		cmd = OCFS2_IOC_UNRESVSP64;
>  

[PATCH V2] ocfs2: pass u64 to ocfs2_truncate_inline maybe overflow

[Edward Adam Davis]

Syzbot reported a kernel BUG in ocfs2_truncate_inline.
There are two reasons for this: first, the parameter value passed is greater
than UINT_MAX, second, the start and end parameters of ocfs2_truncate_inline
are "unsigned int".

So, we need to add a sanity check for byte_start and byte_len right before
ocfs2_truncate_inline() in ocfs2_remove_inode_range(), if they are greater
than UINT_MAX return -EFBIG.

Reported-by: syzbot+81092778aac03460d6b7@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=81092778aac03460d6b7
Signed-off-by: Edward Adam Davis <eadavis@qq.com>
---
V1 -> V2: move sanity check to ocfs2_remove_inode_range

 fs/ocfs2/file.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/fs/ocfs2/file.c b/fs/ocfs2/file.c
index ad131a2fc58e..05d6a8acfcda 100644
--- a/fs/ocfs2/file.c
+++ b/fs/ocfs2/file.c
@@ -1784,6 +1784,11 @@ int ocfs2_remove_inode_range(struct inode *inode,
 		return 0;
 
 	if (OCFS2_I(inode)->ip_dyn_features & OCFS2_INLINE_DATA_FL) {
+		if (byte_start > UINT_MAX || byte_start + byte_len > UINT_MAX) {
+			ret = -EFBIG;
+			mlog_errno(ret);
+			goto out;
+		}
 		ret = ocfs2_truncate_inline(inode, di_bh, byte_start,
 					    byte_start + byte_len, 0);
 		if (ret) {
-- 
2.43.0

Re: [ocfs2?] kernel BUG in ocfs2_truncate_inline

[Edward Adam Davis]

offset or offset + len greater than UINT_MAX, if true, it will overflow in ocfs2_truncate_inline

#syz test

diff --git a/fs/ocfs2/file.c b/fs/ocfs2/file.c
index ad131a2fc58e..05d6a8acfcda 100644
--- a/fs/ocfs2/file.c
+++ b/fs/ocfs2/file.c
@@ -1784,6 +1784,11 @@ int ocfs2_remove_inode_range(struct inode *inode,
 		return 0;
 
 	if (OCFS2_I(inode)->ip_dyn_features & OCFS2_INLINE_DATA_FL) {
+		if (byte_start > UINT_MAX || byte_start + byte_len > UINT_MAX) {
+			ret = -EFBIG;
+			mlog_errno(ret);
+			goto out;
+		}
 		ret = ocfs2_truncate_inline(inode, di_bh, byte_start,
 					    byte_start + byte_len, 0);
 		if (ret) {

Re: [syzbot] [ocfs2?] kernel BUG in ocfs2_truncate_inline

[syzbot]

Hello,

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

Reported-by: syzbot+81092778aac03460d6b7@syzkaller.appspotmail.com
Tested-by: syzbot+81092778aac03460d6b7@syzkaller.appspotmail.com

Tested on:

commit:         d3d15566 Merge tag 'mm-hotfixes-stable-2024-10-09-15-4..
git tree:       upstream
console output: https://syzkaller.appspot.com/x/log.txt?x=1683cb27980000
kernel config:  https://syzkaller.appspot.com/x/.config?x=7a3fccdd0bb995
dashboard link: https://syzkaller.appspot.com/bug?extid=81092778aac03460d6b7
compiler:       Debian clang version 15.0.6, GNU ld (GNU Binutils for Debian) 2.40
patch:          https://syzkaller.appspot.com/x/patch.diff?x=17c36fd0580000

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

Re: [PATCH V2] ocfs2: pass u64 to ocfs2_truncate_inline maybe overflow

[Su Yue]

On Thu 10 Oct 2024 at 22:31, Edward Adam Davis <eadavis@qq.com> 
wrote:

> Syzbot reported a kernel BUG in ocfs2_truncate_inline.
> There are two reasons for this: first, the parameter value 
> passed is greater
> than UINT_MAX, second, the start and end parameters of 
> ocfs2_truncate_inline
> are "unsigned int".
>
> So, we need to add a sanity check for byte_start and byte_len 
> right before
> ocfs2_truncate_inline() in ocfs2_remove_inode_range(), if they 
> are greater
> than UINT_MAX return -EFBIG.
>
> Reported-by: 
> syzbot+81092778aac03460d6b7@syzkaller.appspotmail.com
> Closes: 
> https://syzkaller.appspot.com/bug?extid=81092778aac03460d6b7
> Signed-off-by: Edward Adam Davis <eadavis@qq.com>
> ---
> V1 -> V2: move sanity check to ocfs2_remove_inode_range
>
>  fs/ocfs2/file.c | 5 +++++
>  1 file changed, 5 insertions(+)
>
> diff --git a/fs/ocfs2/file.c b/fs/ocfs2/file.c
> index ad131a2fc58e..05d6a8acfcda 100644
> --- a/fs/ocfs2/file.c
> +++ b/fs/ocfs2/file.c
> @@ -1784,6 +1784,11 @@ int ocfs2_remove_inode_range(struct inode 
> *inode,
>  		return 0;
>
>  	if (OCFS2_I(inode)->ip_dyn_features & OCFS2_INLINE_DATA_FL) {
> +		if (byte_start > UINT_MAX || byte_start + byte_len > 
> UINT_MAX) {
>
Why not use ocfs2_max_inline_data_with_xattr() here? Yes, UINT_MAX 
indeed
solves overflow problem Syzbot reported but you can find much 
lowerer
limit if once looked into inline data structures.
Also, ocfs2_truncate_inline() can be enhanced e.g. replace
BUG_ON(start > end) with error out.

--
Su


> +			ret = -EFBIG;
> +			mlog_errno(ret);
> +			goto out;
> +		}
>  		ret = ocfs2_truncate_inline(inode, di_bh, byte_start,
>  					    byte_start + byte_len, 0);
>  		if (ret) {

Re: [PATCH V2] ocfs2: pass u64 to ocfs2_truncate_inline maybe overflow

[Joseph Qi]

On 10/11/24 9:07 AM, Su Yue wrote:
> 
> On Thu 10 Oct 2024 at 22:31, Edward Adam Davis <eadavis@qq.com> wrote:
> 
>> Syzbot reported a kernel BUG in ocfs2_truncate_inline.
>> There are two reasons for this: first, the parameter value passed is greater
>> than UINT_MAX, second, the start and end parameters of ocfs2_truncate_inline
>> are "unsigned int".
>>
>> So, we need to add a sanity check for byte_start and byte_len right before
>> ocfs2_truncate_inline() in ocfs2_remove_inode_range(), if they are greater
>> than UINT_MAX return -EFBIG.
>>
>> Reported-by: syzbot+81092778aac03460d6b7@syzkaller.appspotmail.com
>> Closes: https://syzkaller.appspot.com/bug?extid=81092778aac03460d6b7
>> Signed-off-by: Edward Adam Davis <eadavis@qq.com>
>> ---
>> V1 -> V2: move sanity check to ocfs2_remove_inode_range
>>
>>  fs/ocfs2/file.c | 5 +++++
>>  1 file changed, 5 insertions(+)
>>
>> diff --git a/fs/ocfs2/file.c b/fs/ocfs2/file.c
>> index ad131a2fc58e..05d6a8acfcda 100644
>> --- a/fs/ocfs2/file.c
>> +++ b/fs/ocfs2/file.c
>> @@ -1784,6 +1784,11 @@ int ocfs2_remove_inode_range(struct inode *inode,
>>          return 0;
>>
>>      if (OCFS2_I(inode)->ip_dyn_features & OCFS2_INLINE_DATA_FL) {
>> +        if (byte_start > UINT_MAX || byte_start + byte_len > UINT_MAX) {
>>
> Why not use ocfs2_max_inline_data_with_xattr() here? Yes, UINT_MAX indeed
> solves overflow problem Syzbot reported but you can find much lowerer
> limit if once looked into inline data structures.

Right, since it is inline data, so the offset can't exceeds block size
at least. You can refer bad inline data check in
ocfs2_read_inline_data().

Thanks,
Joseph
 

Re: [PATCH V2] ocfs2: pass u64 to ocfs2_truncate_inline maybe overflow

[Joseph Qi]

On 10/11/24 10:01 AM, Joseph Qi wrote:
> 
> 
> On 10/11/24 9:07 AM, Su Yue wrote:
>>
>> On Thu 10 Oct 2024 at 22:31, Edward Adam Davis <eadavis@qq.com> wrote:
>>
>>> Syzbot reported a kernel BUG in ocfs2_truncate_inline.
>>> There are two reasons for this: first, the parameter value passed is greater
>>> than UINT_MAX, second, the start and end parameters of ocfs2_truncate_inline
>>> are "unsigned int".
>>>
>>> So, we need to add a sanity check for byte_start and byte_len right before
>>> ocfs2_truncate_inline() in ocfs2_remove_inode_range(), if they are greater
>>> than UINT_MAX return -EFBIG.
>>>
>>> Reported-by: syzbot+81092778aac03460d6b7@syzkaller.appspotmail.com
>>> Closes: https://syzkaller.appspot.com/bug?extid=81092778aac03460d6b7
>>> Signed-off-by: Edward Adam Davis <eadavis@qq.com>
>>> ---
>>> V1 -> V2: move sanity check to ocfs2_remove_inode_range
>>>
>>>  fs/ocfs2/file.c | 5 +++++
>>>  1 file changed, 5 insertions(+)
>>>
>>> diff --git a/fs/ocfs2/file.c b/fs/ocfs2/file.c
>>> index ad131a2fc58e..05d6a8acfcda 100644
>>> --- a/fs/ocfs2/file.c
>>> +++ b/fs/ocfs2/file.c
>>> @@ -1784,6 +1784,11 @@ int ocfs2_remove_inode_range(struct inode *inode,
>>>          return 0;
>>>
>>>      if (OCFS2_I(inode)->ip_dyn_features & OCFS2_INLINE_DATA_FL) {
>>> +        if (byte_start > UINT_MAX || byte_start + byte_len > UINT_MAX) {
>>>
>> Why not use ocfs2_max_inline_data_with_xattr() here? Yes, UINT_MAX indeed
>> solves overflow problem Syzbot reported but you can find much lowerer
>> limit if once looked into inline data structures.
> 
> Right, since it is inline data, so the offset can't exceeds block size
> at least. You can refer bad inline data check in
> ocfs2_read_inline_data().
> 
Could you please update the check condition and send a new version?

Thanks,
Joseph

[PATCH V3] ocfs2: pass u64 to ocfs2_truncate_inline maybe overflow

[Edward Adam Davis]

Syzbot reported a kernel BUG in ocfs2_truncate_inline.
There are two reasons for this: first, the parameter value passed is greater
than UINT_MAX, second, the start and end parameters of ocfs2_truncate_inline
are "unsigned int".

So, we need to add a sanity check for byte_start and byte_len right before
ocfs2_truncate_inline() in ocfs2_remove_inode_range(), if they are greater
than ocfs2_max_inline_data_with_xattr return -EFBIG.

Reported-by: syzbot+81092778aac03460d6b7@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=81092778aac03460d6b7
Signed-off-by: Edward Adam Davis <eadavis@qq.com>
---
V1 -> V2: move sanity check to ocfs2_remove_inode_range
V2 -> V3: use ocfs2_max_inline_data_with_xattr return value replace UINT_MAX

 fs/ocfs2/file.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/fs/ocfs2/file.c b/fs/ocfs2/file.c
index ad131a2fc58e..9327aa2f1bf4 100644
--- a/fs/ocfs2/file.c
+++ b/fs/ocfs2/file.c
@@ -1784,6 +1784,12 @@ int ocfs2_remove_inode_range(struct inode *inode,
 		return 0;
 
 	if (OCFS2_I(inode)->ip_dyn_features & OCFS2_INLINE_DATA_FL) {
+		int max_inl = ocfs2_max_inline_data_with_xattr(inode->i_sb, di);
+		if (byte_start > max_inl || byte_start + byte_len > max_inl) {
+			ret = -EFBIG;
+			mlog_errno(ret);
+			goto out;
+		}
 		ret = ocfs2_truncate_inline(inode, di_bh, byte_start,
 					    byte_start + byte_len, 0);
 		if (ret) {
-- 
2.43.0

Re: [ocfs2?] kernel BUG in ocfs2_truncate_inline

[Edward Adam Davis]

offset or offset + len greater than inline data max size, if true, it will
overflow in ocfs2_truncate_inline.

#syz test

diff --git a/fs/ocfs2/file.c b/fs/ocfs2/file.c
index ad131a2fc58e..9327aa2f1bf4 100644
--- a/fs/ocfs2/file.c
+++ b/fs/ocfs2/file.c
@@ -1784,6 +1784,12 @@ int ocfs2_remove_inode_range(struct inode *inode,
 		return 0;
 
 	if (OCFS2_I(inode)->ip_dyn_features & OCFS2_INLINE_DATA_FL) {
+		int max_inl = ocfs2_max_inline_data_with_xattr(inode->i_sb, di);
+		if (byte_start > max_inl || byte_start + byte_len > max_inl) {
+			ret = -EFBIG;
+			mlog_errno(ret);
+			goto out;
+		}
 		ret = ocfs2_truncate_inline(inode, di_bh, byte_start,
 					    byte_start + byte_len, 0);
 		if (ret) {

Re: [syzbot] [ocfs2?] kernel BUG in ocfs2_truncate_inline

[syzbot]

Hello,

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

Reported-by: syzbot+81092778aac03460d6b7@syzkaller.appspotmail.com
Tested-by: syzbot+81092778aac03460d6b7@syzkaller.appspotmail.com

Tested on:

commit:         2f87d091 Merge tag 'trace-ringbuffer-v6.12-rc3' of git..
git tree:       upstream
console output: https://syzkaller.appspot.com/x/log.txt?x=1256b727980000
kernel config:  https://syzkaller.appspot.com/x/.config?x=164d2822debd8b0d
dashboard link: https://syzkaller.appspot.com/bug?extid=81092778aac03460d6b7
compiler:       Debian clang version 15.0.6, GNU ld (GNU Binutils for Debian) 2.40
patch:          https://syzkaller.appspot.com/x/patch.diff?x=1770d887980000

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

Re: [PATCH V3] ocfs2: pass u64 to ocfs2_truncate_inline maybe overflow

[Joseph Qi]

On 10/16/24 11:22 AM, Edward Adam Davis wrote:
> Syzbot reported a kernel BUG in ocfs2_truncate_inline.
> There are two reasons for this: first, the parameter value passed is greater
> than UINT_MAX, second, the start and end parameters of ocfs2_truncate_inline
> are "unsigned int".
> 
> So, we need to add a sanity check for byte_start and byte_len right before
> ocfs2_truncate_inline() in ocfs2_remove_inode_range(), if they are greater
> than ocfs2_max_inline_data_with_xattr return -EFBIG.
> 
> Reported-by: syzbot+81092778aac03460d6b7@syzkaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=81092778aac03460d6b7
> Signed-off-by: Edward Adam Davis <eadavis@qq.com>
> ---
> V1 -> V2: move sanity check to ocfs2_remove_inode_range
> V2 -> V3: use ocfs2_max_inline_data_with_xattr return value replace UINT_MAX
> 
>  fs/ocfs2/file.c | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/fs/ocfs2/file.c b/fs/ocfs2/file.c
> index ad131a2fc58e..9327aa2f1bf4 100644
> --- a/fs/ocfs2/file.c
> +++ b/fs/ocfs2/file.c
> @@ -1784,6 +1784,12 @@ int ocfs2_remove_inode_range(struct inode *inode,
>  		return 0;
>  
>  	if (OCFS2_I(inode)->ip_dyn_features & OCFS2_INLINE_DATA_FL) {
> +		int max_inl = ocfs2_max_inline_data_with_xattr(inode->i_sb, di);

Or rename it to 'id_count' refer to 'struct ocfs2_inline_data'.
Better to leave a blank line here.

> +		if (byte_start > max_inl || byte_start + byte_len > max_inl) {
> +			ret = -EFBIG;

Seems 'EINVAL' is more proper here.
Please do corresponding change in commit log.

> +			mlog_errno(ret);
> +			goto out;
> +		}

Better to leave a blank line.

Thanks,
Joseph

>  		ret = ocfs2_truncate_inline(inode, di_bh, byte_start,
>  					    byte_start + byte_len, 0);
>  		if (ret) {

[PATCH V4] ocfs2: pass u64 to ocfs2_truncate_inline maybe overflow

[Edward Adam Davis]

Syzbot reported a kernel BUG in ocfs2_truncate_inline.
There are two reasons for this: first, the parameter value passed is greater
than ocfs2_max_inline_data_with_xattr, second, the start and end parameters
of ocfs2_truncate_inline are "unsigned int".

So, we need to add a sanity check for byte_start and byte_len right before
ocfs2_truncate_inline() in ocfs2_remove_inode_range(), if they are greater
than ocfs2_max_inline_data_with_xattr return -EINVAL.

Reported-by: syzbot+81092778aac03460d6b7@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=81092778aac03460d6b7
Signed-off-by: Edward Adam Davis <eadavis@qq.com>
---
V1 -> V2: move sanity check to ocfs2_remove_inode_range
V2 -> V3: use ocfs2_max_inline_data_with_xattr return value replace UINT_MAX
V3 -> V4: rename variable, modify return value and comments

 fs/ocfs2/file.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/fs/ocfs2/file.c b/fs/ocfs2/file.c
index ad131a2fc58e..47121ee4b4df 100644
--- a/fs/ocfs2/file.c
+++ b/fs/ocfs2/file.c
@@ -1784,6 +1784,14 @@ int ocfs2_remove_inode_range(struct inode *inode,
 		return 0;
 
 	if (OCFS2_I(inode)->ip_dyn_features & OCFS2_INLINE_DATA_FL) {
+		int id_count = ocfs2_max_inline_data_with_xattr(inode->i_sb, di);
+
+		if (byte_start > id_count || byte_start + byte_len > id_count) {
+			ret = -EINVAL;
+			mlog_errno(ret);
+			goto out;
+		}
+
 		ret = ocfs2_truncate_inline(inode, di_bh, byte_start,
 					    byte_start + byte_len, 0);
 		if (ret) {
-- 
2.43.0

Re: [PATCH V4] ocfs2: pass u64 to ocfs2_truncate_inline maybe overflow

[Joseph Qi]

On 10/16/24 7:43 PM, Edward Adam Davis wrote:
> Syzbot reported a kernel BUG in ocfs2_truncate_inline.
> There are two reasons for this: first, the parameter value passed is greater
> than ocfs2_max_inline_data_with_xattr, second, the start and end parameters
> of ocfs2_truncate_inline are "unsigned int".
> 
> So, we need to add a sanity check for byte_start and byte_len right before
> ocfs2_truncate_inline() in ocfs2_remove_inode_range(), if they are greater
> than ocfs2_max_inline_data_with_xattr return -EINVAL.
> 
> Reported-by: syzbot+81092778aac03460d6b7@syzkaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=81092778aac03460d6b7
> Signed-off-by: Edward Adam Davis <eadavis@qq.com>

Looks fine.
Reviewed-by: Joseph Qi <joseph.qi@linux.alibaba.com>

> ---
> V1 -> V2: move sanity check to ocfs2_remove_inode_range
> V2 -> V3: use ocfs2_max_inline_data_with_xattr return value replace UINT_MAX
> V3 -> V4: rename variable, modify return value and comments
> 
>  fs/ocfs2/file.c | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/fs/ocfs2/file.c b/fs/ocfs2/file.c
> index ad131a2fc58e..47121ee4b4df 100644
> --- a/fs/ocfs2/file.c
> +++ b/fs/ocfs2/file.c
> @@ -1784,6 +1784,14 @@ int ocfs2_remove_inode_range(struct inode *inode,
>  		return 0;
>  
>  	if (OCFS2_I(inode)->ip_dyn_features & OCFS2_INLINE_DATA_FL) {
> +		int id_count = ocfs2_max_inline_data_with_xattr(inode->i_sb, di);
> +
> +		if (byte_start > id_count || byte_start + byte_len > id_count) {
> +			ret = -EINVAL;
> +			mlog_errno(ret);
> +			goto out;
> +		}
> +
>  		ret = ocfs2_truncate_inline(inode, di_bh, byte_start,
>  					    byte_start + byte_len, 0);
>  		if (ret) {

Re: [PATCH V4] ocfs2: pass u64 to ocfs2_truncate_inline maybe overflow

[Joseph Qi]

On 10/16/24 7:47 PM, Joseph Qi wrote:
> 
> 
> On 10/16/24 7:43 PM, Edward Adam Davis wrote:
>> Syzbot reported a kernel BUG in ocfs2_truncate_inline.
>> There are two reasons for this: first, the parameter value passed is greater
>> than ocfs2_max_inline_data_with_xattr, second, the start and end parameters
>> of ocfs2_truncate_inline are "unsigned int".
>>
>> So, we need to add a sanity check for byte_start and byte_len right before
>> ocfs2_truncate_inline() in ocfs2_remove_inode_range(), if they are greater
>> than ocfs2_max_inline_data_with_xattr return -EINVAL.
>>
>> Reported-by: syzbot+81092778aac03460d6b7@syzkaller.appspotmail.com
>> Closes: https://syzkaller.appspot.com/bug?extid=81092778aac03460d6b7
>> Signed-off-by: Edward Adam Davis <eadavis@qq.com>
> 
> Looks fine.
> Reviewed-by: Joseph Qi <joseph.qi@linux.alibaba.com>
> 

Fixes: 1afc32b95233 ("ocfs2: Write support for inline data")
Cc: <stable@vger.kernel.org>

>> ---
>> V1 -> V2: move sanity check to ocfs2_remove_inode_range
>> V2 -> V3: use ocfs2_max_inline_data_with_xattr return value replace UINT_MAX
>> V3 -> V4: rename variable, modify return value and comments
>>
>>  fs/ocfs2/file.c | 8 ++++++++
>>  1 file changed, 8 insertions(+)
>>
>> diff --git a/fs/ocfs2/file.c b/fs/ocfs2/file.c
>> index ad131a2fc58e..47121ee4b4df 100644
>> --- a/fs/ocfs2/file.c
>> +++ b/fs/ocfs2/file.c
>> @@ -1784,6 +1784,14 @@ int ocfs2_remove_inode_range(struct inode *inode,
>>  		return 0;
>>  
>>  	if (OCFS2_I(inode)->ip_dyn_features & OCFS2_INLINE_DATA_FL) {
>> +		int id_count = ocfs2_max_inline_data_with_xattr(inode->i_sb, di);
>> +
>> +		if (byte_start > id_count || byte_start + byte_len > id_count) {
>> +			ret = -EINVAL;
>> +			mlog_errno(ret);
>> +			goto out;
>> +		}
>> +
>>  		ret = ocfs2_truncate_inline(inode, di_bh, byte_start,
>>  					    byte_start + byte_len, 0);
>>  		if (ret) {
>