jfs: fix array-index-out-of-bounds in dtInsertEntry

[PATCH] jfs: fix array-index-out-of-bounds in dtInsertEntry

Posted by Ghanshyam Agrawal 1 month, 2 weeks ago

The value of p->header.freelist can be less than zero which
causes an error in dtInsertEntry. Added a check in dtInsert
to address it.

Reported-by: syzbot+5f7f0caf9979e9d09ff8@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=5f7f0caf9979e9d09ff8
Signed-off-by: Ghanshyam Agrawal <ghanshyam1898@gmail.com>
---
 fs/jfs/jfs_dtree.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/jfs/jfs_dtree.c b/fs/jfs/jfs_dtree.c
index 5d3127ca68a4..51bb3e14551b 100644
--- a/fs/jfs/jfs_dtree.c
+++ b/fs/jfs/jfs_dtree.c
@@ -834,7 +834,7 @@ int dtInsert(tid_t tid, struct inode *ip,
 	 * the full page.
 	 */
 	DT_GETSEARCH(ip, btstack->top, bn, mp, p, index);
-	if (p->header.freelist == 0)
+	if (p->header.freelist <= 0)
 		return -EINVAL;
 
 	/*
-- 
2.34.1

Re: [PATCH] jfs: fix array-index-out-of-bounds in dtInsertEntry

Posted by Dave Kleikamp 4 weeks ago

On 10/10/24 8:43AM, Ghanshyam Agrawal wrote:
> The value of p->header.freelist can be less than zero which
> causes an error in dtInsertEntry. Added a check in dtInsert
> to address it.
> 
> Reported-by: syzbot+5f7f0caf9979e9d09ff8@syzkaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=5f7f0caf9979e9d09ff8
> Signed-off-by: Ghanshyam Agrawal <ghanshyam1898@gmail.com>

Looks good. I'll apply this one.

> ---
>   fs/jfs/jfs_dtree.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/fs/jfs/jfs_dtree.c b/fs/jfs/jfs_dtree.c
> index 5d3127ca68a4..51bb3e14551b 100644
> --- a/fs/jfs/jfs_dtree.c
> +++ b/fs/jfs/jfs_dtree.c
> @@ -834,7 +834,7 @@ int dtInsert(tid_t tid, struct inode *ip,
>   	 * the full page.
>   	 */
>   	DT_GETSEARCH(ip, btstack->top, bn, mp, p, index);
> -	if (p->header.freelist == 0)
> +	if (p->header.freelist <= 0)
>   		return -EINVAL;
>   
>   	/*

Re: [PATCH] jfs: fix array-index-out-of-bounds in dtInsertEntry

Posted by Dave Kleikamp 3 weeks, 6 days ago

On 10/29/24 6:03PM, Dave Kleikamp wrote:
> On 10/10/24 8:43AM, Ghanshyam Agrawal wrote:
>> The value of p->header.freelist can be less than zero which
>> causes an error in dtInsertEntry. Added a check in dtInsert
>> to address it.
>>
>> Reported-by: syzbot+5f7f0caf9979e9d09ff8@syzkaller.appspotmail.com
>> Closes: https://syzkaller.appspot.com/bug?extid=5f7f0caf9979e9d09ff8
>> Signed-off-by: Ghanshyam Agrawal <ghanshyam1898@gmail.com>
> 
> Looks good. I'll apply this one.

Unapplying it. This caused regressions running xfstests. I'll need to 
look into it more carefully.

Shaggy

> 
>> ---
>>   fs/jfs/jfs_dtree.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/fs/jfs/jfs_dtree.c b/fs/jfs/jfs_dtree.c
>> index 5d3127ca68a4..51bb3e14551b 100644
>> --- a/fs/jfs/jfs_dtree.c
>> +++ b/fs/jfs/jfs_dtree.c
>> @@ -834,7 +834,7 @@ int dtInsert(tid_t tid, struct inode *ip,
>>        * the full page.
>>        */
>>       DT_GETSEARCH(ip, btstack->top, bn, mp, p, index);
>> -    if (p->header.freelist == 0)
>> +    if (p->header.freelist <= 0)
>>           return -EINVAL;
>>       /*

Re: [PATCH] jfs: fix array-index-out-of-bounds in dtInsertEntry

Posted by Ghanshyam Agrawal 3 weeks, 6 days ago

On Thu, Oct 31, 2024 at 3:09 AM Dave Kleikamp <dave.kleikamp@oracle.com> wrote:
>
> On 10/29/24 6:03PM, Dave Kleikamp wrote:
> > On 10/10/24 8:43AM, Ghanshyam Agrawal wrote:
> >> The value of p->header.freelist can be less than zero which
> >> causes an error in dtInsertEntry. Added a check in dtInsert
> >> to address it.
> >>
> >> Reported-by: syzbot+5f7f0caf9979e9d09ff8@syzkaller.appspotmail.com
> >> Closes: https://syzkaller.appspot.com/bug?extid=5f7f0caf9979e9d09ff8
> >> Signed-off-by: Ghanshyam Agrawal <ghanshyam1898@gmail.com>
> >
> > Looks good. I'll apply this one.
>
> Unapplying it. This caused regressions running xfstests. I'll need to
> look into it more carefully.
>
> Shaggy
>
> >
> >> ---
> >>   fs/jfs/jfs_dtree.c | 2 +-
> >>   1 file changed, 1 insertion(+), 1 deletion(-)
> >>
> >> diff --git a/fs/jfs/jfs_dtree.c b/fs/jfs/jfs_dtree.c
> >> index 5d3127ca68a4..51bb3e14551b 100644
> >> --- a/fs/jfs/jfs_dtree.c
> >> +++ b/fs/jfs/jfs_dtree.c
> >> @@ -834,7 +834,7 @@ int dtInsert(tid_t tid, struct inode *ip,
> >>        * the full page.
> >>        */
> >>       DT_GETSEARCH(ip, btstack->top, bn, mp, p, index);
> >> -    if (p->header.freelist == 0)
> >> +    if (p->header.freelist <= 0)
> >>           return -EINVAL;
> >>       /*
>

Hello Dave,

Thank you for reviewing and testing my patch. Let me go through the
xfstests results, find the issue and send a v2 for this patch.

Thanks & Regards,
Ghanshyam Agrawal