diff mbox series

jfs: Fix array-index-out-of-bounds in diFree

Message ID 20240424172240.148883-1-aha310510@gmail.com (mailing list archive)
State New
Headers show
Series jfs: Fix array-index-out-of-bounds in diFree | expand

Commit Message

Jeongjun Park April 24, 2024, 5:22 p.m. UTC
[syzbot report]
UBSAN: array-index-out-of-bounds in fs/jfs/jfs_imap.c:886:2
index 524288 is out of range for type 'struct mutex[128]'
CPU: 0 PID: 113 Comm: jfsCommit Not tainted 6.9.0-rc5-syzkaller-00036-g9d1ddab261f3 #0
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 03/27/2024
Call Trace:
 <TASK>
 __dump_stack lib/dump_stack.c:88 [inline]
 dump_stack_lvl+0x241/0x360 lib/dump_stack.c:114
 ubsan_epilogue lib/ubsan.c:231 [inline]
 __ubsan_handle_out_of_bounds+0x121/0x150 lib/ubsan.c:429
 diFree+0x21c3/0x2fb0 fs/jfs/jfs_imap.c:886
 jfs_evict_inode+0x32d/0x440 fs/jfs/inode.c:156
 evict+0x2a8/0x630 fs/inode.c:667
 txUpdateMap+0x829/0x9f0 fs/jfs/jfs_txnmgr.c:2367
 txLazyCommit fs/jfs/jfs_txnmgr.c:2664 [inline]
 jfs_lazycommit+0x49a/0xb80 fs/jfs/jfs_txnmgr.c:2733
 kthread+0x2f0/0x390 kernel/kthread.c:388
 ret_from_fork+0x4b/0x80 arch/x86/kernel/process.c:147
 ret_from_fork_asm+0x1a/0x30 arch/x86/entry/entry_64.S:244
 </TASK>
---[ end trace ]---
Kernel panic - not syncing: UBSAN: panic_on_warn set ...
CPU: 1 PID: 113 Comm: jfsCommit Not tainted 6.9.0-rc5-syzkaller-00036-g9d1ddab261f3 #0
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 03/27/2024
Call Trace:
 <TASK>
 __dump_stack lib/dump_stack.c:88 [inline]
 dump_stack_lvl+0x241/0x360 lib/dump_stack.c:114
 panic+0x349/0x860 kernel/panic.c:348
 check_panic_on_warn+0x86/0xb0 kernel/panic.c:241
 ubsan_epilogue lib/ubsan.c:236 [inline]
 __ubsan_handle_out_of_bounds+0x141/0x150 lib/ubsan.c:429
 diFree+0x21c3/0x2fb0 fs/jfs/jfs_imap.c:886
 jfs_evict_inode+0x32d/0x440 fs/jfs/inode.c:156
 evict+0x2a8/0x630 fs/inode.c:667
 txUpdateMap+0x829/0x9f0 fs/jfs/jfs_txnmgr.c:2367
 txLazyCommit fs/jfs/jfs_txnmgr.c:2664 [inline]
 jfs_lazycommit+0x49a/0xb80 fs/jfs/jfs_txnmgr.c:2733
 kthread+0x2f0/0x390 kernel/kthread.c:388
 ret_from_fork+0x4b/0x80 arch/x86/kernel/process.c:147
 ret_from_fork_asm+0x1a/0x30 arch/x86/entry/entry_64.S:244
 </TASK>
Kernel Offset: disabled
Rebooting in 86400 seconds..
===========================================================

Due to overflow, a value that is too large is entered into the agno 
value. Therefore, we need to add code to check the agno value.

Reported-by: syzbot+241c815bda521982cb49@syzkaller.appspotmail.com
Signed-off-by: Jeongjun Park <aha310510@gmail.com>
---
 fs/jfs/jfs_imap.c | 5 +++++
 1 file changed, 5 insertions(+)

Comments

Matthew Wilcox April 24, 2024, 5:41 p.m. UTC | #1
On Thu, Apr 25, 2024 at 02:22:40AM +0900, Jeongjun Park wrote:
> Due to overflow, a value that is too large is entered into the agno 
> value. Therefore, we need to add code to check the agno value.

This is clearly wrong.

#define BLKTOAG(b,sbi)  ((b) >> ((sbi)->bmap->db_agl2size))

I'd suggest that something has either corrupted the sbi->bmap
pointer or the sbi->bmap->db_agl2size value.

All your patch does is cover up the problem, not fix it.

> Reported-by: syzbot+241c815bda521982cb49@syzkaller.appspotmail.com
> Signed-off-by: Jeongjun Park <aha310510@gmail.com>
> ---
>  fs/jfs/jfs_imap.c | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/fs/jfs/jfs_imap.c b/fs/jfs/jfs_imap.c
> index 2ec35889ad24..0aac083bc0db 100644
> --- a/fs/jfs/jfs_imap.c
> +++ b/fs/jfs/jfs_imap.c
> @@ -881,6 +881,11 @@ int diFree(struct inode *ip)
>  	 */
>  	agno = BLKTOAG(JFS_IP(ip)->agstart, JFS_SBI(ip->i_sb));
>  
> +	if(agno >= MAXAG || agno < 0){
> +		jfs_error(ip->i_sb, "invalid array index (0 <= agno < MAXAG), agno = %d\n", agno);
> +		return -ENOMEM;
> +	}
> +
>  	/* Lock the AG specific inode map information
>  	 */
>  	AG_LOCK(imap, agno);
> -- 
> 2.34.1
>
Jeongjun Park April 25, 2024, 12:44 p.m. UTC | #2
Through direct testing and debugging, I've determined that this 
vulnerability occurs when mounting an incorrect image, leading to 
the potential passing of an excessively large value to 
'sbi->bmap->db_agl2size'. Importantly, there have been no instances 
of memory corruption observed within 'sbi->bmap->db_agl2size'. 

Therefore, I think implementing a patch that terminates the 
function in cases where an invalid value is detected.

Thanks.
Matthew Wilcox April 25, 2024, 12:53 p.m. UTC | #3
On Thu, Apr 25, 2024 at 09:44:33PM +0900, Jeongjun Park wrote:
> Through direct testing and debugging, I've determined that this 
> vulnerability occurs when mounting an incorrect image, leading to 
> the potential passing of an excessively large value to 
> 'sbi->bmap->db_agl2size'. Importantly, there have been no instances 
> of memory corruption observed within 'sbi->bmap->db_agl2size'. 
> 
> Therefore, I think implementing a patch that terminates the 
> function in cases where an invalid value is detected.

If that's the problem then the correct place to detect & reject this is
during mount, not at inode free time.
Jeongjun Park April 25, 2024, 2:10 p.m. UTC | #4
Matthew Wilcox wrote:
> If that's the problem then the correct place to detect & reject this is
> during mount, not at inode free time.

I fixed the patch as you said. If you patch in this way, the 
file system will not be affected by the vulnerability at all 
due to the code structure.

Thanks.

---
 fs/jfs/jfs_imap.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/fs/jfs/jfs_imap.c b/fs/jfs/jfs_imap.c
index 2ec35889ad24..ba0aa2f145cc 100644
--- a/fs/jfs/jfs_imap.c
+++ b/fs/jfs/jfs_imap.c
@@ -290,7 +290,7 @@ int diSync(struct inode *ipimap)
 int diRead(struct inode *ip)
 {
 	struct jfs_sb_info *sbi = JFS_SBI(ip->i_sb);
-	int iagno, ino, extno, rc;
+	int iagno, ino, extno, rc, agno;
 	struct inode *ipimap;
 	struct dinode *dp;
 	struct iag *iagp;
@@ -339,6 +339,9 @@ int diRead(struct inode *ip)
 
 	/* get the ag for the iag */
 	agstart = le64_to_cpu(iagp->agstart);
+	agno = BLKTOAG(agstart, JFS_SBI(ip->i_sb));
+	if(agno >= MAXAG || agno < 0)
+		return -EIO;
 
 	release_metapage(mp);
Dave Kleikamp April 25, 2024, 2:14 p.m. UTC | #5
On 4/25/24 9:10AM, Jeongjun Park wrote:
> Matthew Wilcox wrote:
>> If that's the problem then the correct place to detect & reject this is
>> during mount, not at inode free time.
> 
> I fixed the patch as you said. If you patch in this way, the
> file system will not be affected by the vulnerability at all
> due to the code structure.
> 
> Thanks.
> 
> ---
>   fs/jfs/jfs_imap.c | 5 ++++-
>   1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/fs/jfs/jfs_imap.c b/fs/jfs/jfs_imap.c
> index 2ec35889ad24..ba0aa2f145cc 100644
> --- a/fs/jfs/jfs_imap.c
> +++ b/fs/jfs/jfs_imap.c
> @@ -290,7 +290,7 @@ int diSync(struct inode *ipimap)
>   int diRead(struct inode *ip)
>   {
>   	struct jfs_sb_info *sbi = JFS_SBI(ip->i_sb);
> -	int iagno, ino, extno, rc;
> +	int iagno, ino, extno, rc, agno;
>   	struct inode *ipimap;
>   	struct dinode *dp;
>   	struct iag *iagp;
> @@ -339,6 +339,9 @@ int diRead(struct inode *ip)
>   
>   	/* get the ag for the iag */
>   	agstart = le64_to_cpu(iagp->agstart);
> +	agno = BLKTOAG(agstart, JFS_SBI(ip->i_sb));
> +	if(agno >= MAXAG || agno < 0)
> +		return -EIO;

That's the right idea, but move the new code after the call to 
release_metapage().
>   
>   	release_metapage(mp);
>
Matthew Wilcox April 25, 2024, 2:17 p.m. UTC | #6
On Thu, Apr 25, 2024 at 11:10:38PM +0900, Jeongjun Park wrote:
> Matthew Wilcox wrote:
> > If that's the problem then the correct place to detect & reject this is
> > during mount, not at inode free time.
> 
> I fixed the patch as you said. If you patch in this way, the 
> file system will not be affected by the vulnerability at all 
> due to the code structure.

It should be checked earlier than this.  There's this code in
dbMount().  Why isn't this catching it?

        bmp->db_agl2size = le32_to_cpu(dbmp_le->dn_agl2size);
        if (bmp->db_agl2size > L2MAXL2SIZE - L2MAXAG ||
            bmp->db_agl2size < 0) {
                err = -EINVAL;
                goto err_release_metapage;
        }

        if (((bmp->db_mapsize - 1) >> bmp->db_agl2size) > MAXAG) {
                err = -EINVAL;
                goto err_release_metapage;
        }


> Thanks.
> 
> ---
>  fs/jfs/jfs_imap.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/fs/jfs/jfs_imap.c b/fs/jfs/jfs_imap.c
> index 2ec35889ad24..ba0aa2f145cc 100644
> --- a/fs/jfs/jfs_imap.c
> +++ b/fs/jfs/jfs_imap.c
> @@ -290,7 +290,7 @@ int diSync(struct inode *ipimap)
>  int diRead(struct inode *ip)
>  {
>  	struct jfs_sb_info *sbi = JFS_SBI(ip->i_sb);
> -	int iagno, ino, extno, rc;
> +	int iagno, ino, extno, rc, agno;
>  	struct inode *ipimap;
>  	struct dinode *dp;
>  	struct iag *iagp;
> @@ -339,6 +339,9 @@ int diRead(struct inode *ip)
>  
>  	/* get the ag for the iag */
>  	agstart = le64_to_cpu(iagp->agstart);
> +	agno = BLKTOAG(agstart, JFS_SBI(ip->i_sb));
> +	if(agno >= MAXAG || agno < 0)
> +		return -EIO;
>  
>  	release_metapage(mp);
>  
> -- 
> 2.34.1
Jeongjun Park April 25, 2024, 2:24 p.m. UTC | #7
Matthew Wilcox wrote:
> It should be checked earlier than this.  There's this code in
> dbMount().  Why isn't this catching it?

This vulnerability occurs because a very large value can be passed 
to iagp->agstart. So that code doesn't prevent the vulnerability.
Dave Kleikamp April 25, 2024, 2:24 p.m. UTC | #8
On 4/25/24 9:17AM, Matthew Wilcox wrote:
> On Thu, Apr 25, 2024 at 11:10:38PM +0900, Jeongjun Park wrote:
>> Matthew Wilcox wrote:
>>> If that's the problem then the correct place to detect & reject this is
>>> during mount, not at inode free time.
>>
>> I fixed the patch as you said. If you patch in this way, the
>> file system will not be affected by the vulnerability at all
>> due to the code structure.
> 
> It should be checked earlier than this.

Right. This is preferable.

>  There's this code in
> dbMount().  Why isn't this catching it?

db_agstart is not checked in this code. That may be the bad data in this 
case.

There are probably dozens more sanity checks that are missing when data 
is first read from the disk.

Shaggy

> 
>          bmp->db_agl2size = le32_to_cpu(dbmp_le->dn_agl2size);
>          if (bmp->db_agl2size > L2MAXL2SIZE - L2MAXAG ||
>              bmp->db_agl2size < 0) {
>                  err = -EINVAL;
>                  goto err_release_metapage;
>          }
> 
>          if (((bmp->db_mapsize - 1) >> bmp->db_agl2size) > MAXAG) {
>                  err = -EINVAL;
>                  goto err_release_metapage;
>          }
> 
> 
>> Thanks.
>>
>> ---
>>   fs/jfs/jfs_imap.c | 5 ++++-
>>   1 file changed, 4 insertions(+), 1 deletion(-)
>>
>> diff --git a/fs/jfs/jfs_imap.c b/fs/jfs/jfs_imap.c
>> index 2ec35889ad24..ba0aa2f145cc 100644
>> --- a/fs/jfs/jfs_imap.c
>> +++ b/fs/jfs/jfs_imap.c
>> @@ -290,7 +290,7 @@ int diSync(struct inode *ipimap)
>>   int diRead(struct inode *ip)
>>   {
>>   	struct jfs_sb_info *sbi = JFS_SBI(ip->i_sb);
>> -	int iagno, ino, extno, rc;
>> +	int iagno, ino, extno, rc, agno;
>>   	struct inode *ipimap;
>>   	struct dinode *dp;
>>   	struct iag *iagp;
>> @@ -339,6 +339,9 @@ int diRead(struct inode *ip)
>>   
>>   	/* get the ag for the iag */
>>   	agstart = le64_to_cpu(iagp->agstart);
>> +	agno = BLKTOAG(agstart, JFS_SBI(ip->i_sb));
>> +	if(agno >= MAXAG || agno < 0)
>> +		return -EIO;
>>   
>>   	release_metapage(mp);
>>   
>> -- 
>> 2.34.1
Jeongjun Park April 25, 2024, 2:37 p.m. UTC | #9
Matthew Wilcox wrote:
> It should be checked earlier than this.  There's this code in
> dbMount().  Why isn't this catching it?

Send final patch. With the patch that modified the location of 
release_metapage(), out-of-bounds vulnerabilities can now be 
sufficiently prevented.

Reported-by: syzbot+241c815bda521982cb49@syzkaller.appspotmail.com
Signed-off-by: Jeongjun Park <aha310510@gmail.com>
---
 fs/jfs/jfs_imap.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/fs/jfs/jfs_imap.c b/fs/jfs/jfs_imap.c
index 2ec35889ad24..cad1798dc892 100644
--- a/fs/jfs/jfs_imap.c
+++ b/fs/jfs/jfs_imap.c
@@ -290,7 +290,7 @@ int diSync(struct inode *ipimap)
 int diRead(struct inode *ip)
 {
 	struct jfs_sb_info *sbi = JFS_SBI(ip->i_sb);
-	int iagno, ino, extno, rc;
+	int iagno, ino, extno, rc, agno;
 	struct inode *ipimap;
 	struct dinode *dp;
 	struct iag *iagp;
@@ -339,8 +339,11 @@ int diRead(struct inode *ip)
 
 	/* get the ag for the iag */
 	agstart = le64_to_cpu(iagp->agstart);
+	agno = BLKTOAG(agstart, JFS_SBI(ip->i_sb));
 
 	release_metapage(mp);
+	if(agno >= MAXAG || agno < 0)
+		return -EIO;
 
 	rel_inode = (ino & (INOSPERPAGE - 1));
 	pageno = blkno >> sbi->l2nbperpage;
Matthew Wilcox April 25, 2024, 5:04 p.m. UTC | #10
On Thu, Apr 25, 2024 at 11:24:34PM +0900, Jeongjun Park wrote:
> Matthew Wilcox wrote:
> > It should be checked earlier than this.  There's this code in
> > dbMount().  Why isn't this catching it?
> 
> This vulnerability occurs because a very large value can be passed 
> to iagp->agstart. So that code doesn't prevent the vulnerability.

In your earlier mail, you said the large value was found in db_agl2size.
If the problem is in agstart then diRead() is the right place to check it.
Jeongjun Park April 26, 2024, 1:42 a.m. UTC | #11
Matthew Wilcox wrote:
> In your earlier mail, you said the large value was found in db_agl2size.
> If the problem is in agstart then diRead() is the right place to check it.

Oh, I was so distracted last time that I wrote the explanation
incorrectly. I'm sorry.

To explain it accurately, if you pass a very large value to agstart
and set the value passed to db_agl2size to be small, it can be
manipulated so that a value greater than MAXAG is output when the
"agstart >> db_agl2size" operation is performed.
This results in an out-of-bounds vulnerability.

And the final patch before is the one that fixes diRead().

Thanks.
Jeongjun Park April 26, 2024, 2:34 a.m. UTC | #12
I forgot to add Dave to the cc, so I'm sending it again.

Send final patch. With the patch that modified the location of
release_metapage(), out-of-bounds vulnerabilities can now be
sufficiently prevented.

Thanks.

Reported-by: syzbot+241c815bda521982cb49@syzkaller.appspotmail.com
Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Signed-off-by: Jeongjun Park <aha310510@gmail.com>
---
 fs/jfs/jfs_imap.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/fs/jfs/jfs_imap.c b/fs/jfs/jfs_imap.c
index 2ec35889ad24..cad1798dc892 100644
--- a/fs/jfs/jfs_imap.c
+++ b/fs/jfs/jfs_imap.c
@@ -290,7 +290,7 @@ int diSync(struct inode *ipimap)
 int diRead(struct inode *ip)
 {
 	struct jfs_sb_info *sbi = JFS_SBI(ip->i_sb);
-	int iagno, ino, extno, rc;
+	int iagno, ino, extno, rc, agno;
 	struct inode *ipimap;
 	struct dinode *dp;
 	struct iag *iagp;
@@ -339,8 +339,11 @@ int diRead(struct inode *ip)
 
 	/* get the ag for the iag */
 	agstart = le64_to_cpu(iagp->agstart);
+	agno = BLKTOAG(agstart, JFS_SBI(ip->i_sb));
 
 	release_metapage(mp);
+	if(agno >= MAXAG || agno < 0)
+		return -EIO;
 
 	rel_inode = (ino & (INOSPERPAGE - 1));
 	pageno = blkno >> sbi->l2nbperpage;
Matthew Wilcox April 26, 2024, 3:27 a.m. UTC | #13
On Fri, Apr 26, 2024 at 11:34:12AM +0900, Jeongjun Park wrote:
> I forgot to add Dave to the cc, so I'm sending it again.
> 
> Send final patch. With the patch that modified the location of
> release_metapage(), out-of-bounds vulnerabilities can now be
> sufficiently prevented.

This is not a good commit message.

> +	if(agno >= MAXAG || agno < 0)

Please follow normal kernel whitespace rules -- one space between 'if'
and the open paren.
Jeongjun Park April 26, 2024, 3:41 a.m. UTC | #14
Matthew Wilcox wrote:
> This is not a good commit message.

> > +	if(agno >= MAXAG || agno < 0)
>
> Please follow normal kernel whitespace rules -- one space between 'if'
> and the open paren.

Has confirmed. This is a patch that re-edited the relevant part to
comply with the rules.

Thanks.

Reported-by: syzbot+241c815bda521982cb49@syzkaller.appspotmail.com
Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Signed-off-by: Jeongjun Park <aha310510@gmail.com>
---
 fs/jfs/jfs_imap.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/fs/jfs/jfs_imap.c b/fs/jfs/jfs_imap.c
index 2ec35889ad24..1407feccbc2d 100644
--- a/fs/jfs/jfs_imap.c
+++ b/fs/jfs/jfs_imap.c
@@ -290,7 +290,7 @@ int diSync(struct inode *ipimap)
 int diRead(struct inode *ip)
 {
 	struct jfs_sb_info *sbi = JFS_SBI(ip->i_sb);
-	int iagno, ino, extno, rc;
+	int iagno, ino, extno, rc, agno;
 	struct inode *ipimap;
 	struct dinode *dp;
 	struct iag *iagp;
@@ -339,8 +339,11 @@ int diRead(struct inode *ip)
 
 	/* get the ag for the iag */
 	agstart = le64_to_cpu(iagp->agstart);
+	agno = BLKTOAG(agstart, JFS_SBI(ip->i_sb));
 
 	release_metapage(mp);
+	if (agno >= MAXAG || agno < 0)
+		return -EIO;
 
 	rel_inode = (ino & (INOSPERPAGE - 1));
 	pageno = blkno >> sbi->l2nbperpage;
Jeongjun Park May 30, 2024, 1:28 p.m. UTC | #15
>
> Matthew Wilcox wrote:
> > This is not a good commit message.
>
> > > +   if(agno >= MAXAG || agno < 0)
> >
> > Please follow normal kernel whitespace rules -- one space between 'if'
> > and the open paren.
>
> Has confirmed. This is a patch that re-edited the relevant part to
> comply with the rules.
>
> Thanks.
>

I have just discovered that the patch I sent last time has been left 
unattended. It appears that the vulnerability continues to occur in 
version 6.10.0-rc1. I would appreciate it if you could review the patch
and let me know what might be wrong with it.

Regards

Reported-by: syzbot+241c815bda521982cb49@syzkaller.appspotmail.com
Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Signed-off-by: Jeongjun Park <aha310510@gmail.com>
---
 fs/jfs/jfs_imap.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/fs/jfs/jfs_imap.c b/fs/jfs/jfs_imap.c
index 2ec35889ad24..1407feccbc2d 100644
--- a/fs/jfs/jfs_imap.c
+++ b/fs/jfs/jfs_imap.c
@@ -290,7 +290,7 @@ int diSync(struct inode *ipimap)
 int diRead(struct inode *ip)
 {
 	struct jfs_sb_info *sbi = JFS_SBI(ip->i_sb);
-	int iagno, ino, extno, rc;
+	int iagno, ino, extno, rc, agno;
 	struct inode *ipimap;
 	struct dinode *dp;
 	struct iag *iagp;
@@ -339,8 +339,11 @@ int diRead(struct inode *ip)
 
 	/* get the ag for the iag */
 	agstart = le64_to_cpu(iagp->agstart);
+	agno = BLKTOAG(agstart, JFS_SBI(ip->i_sb));
 
 	release_metapage(mp);
+	if (agno >= MAXAG || agno < 0)
+		return -EIO;
 
 	rel_inode = (ino & (INOSPERPAGE - 1));
 	pageno = blkno >> sbi->l2nbperpage;
Dave Kleikamp June 26, 2024, 5:40 p.m. UTC | #16
On 5/30/24 8:28AM, Jeongjun Park wrote:
>>
>> Matthew Wilcox wrote:
>>> This is not a good commit message.
>>
>>>> +   if(agno >= MAXAG || agno < 0)
>>>
>>> Please follow normal kernel whitespace rules -- one space between 'if'
>>> and the open paren.
>>
>> Has confirmed. This is a patch that re-edited the relevant part to
>> comply with the rules.
>>
>> Thanks.
>>
> 
> I have just discovered that the patch I sent last time has been left
> unattended. It appears that the vulnerability continues to occur in
> version 6.10.0-rc1. I would appreciate it if you could review the patch
> and let me know what might be wrong with it.

Sorry this has taken me so long.

Applied.

> 
> Regards
> 
> Reported-by: syzbot+241c815bda521982cb49@syzkaller.appspotmail.com
> Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
> Signed-off-by: Jeongjun Park <aha310510@gmail.com>
> ---
>   fs/jfs/jfs_imap.c | 5 ++++-
>   1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/fs/jfs/jfs_imap.c b/fs/jfs/jfs_imap.c
> index 2ec35889ad24..1407feccbc2d 100644
> --- a/fs/jfs/jfs_imap.c
> +++ b/fs/jfs/jfs_imap.c
> @@ -290,7 +290,7 @@ int diSync(struct inode *ipimap)
>   int diRead(struct inode *ip)
>   {
>   	struct jfs_sb_info *sbi = JFS_SBI(ip->i_sb);
> -	int iagno, ino, extno, rc;
> +	int iagno, ino, extno, rc, agno;
>   	struct inode *ipimap;
>   	struct dinode *dp;
>   	struct iag *iagp;
> @@ -339,8 +339,11 @@ int diRead(struct inode *ip)
>   
>   	/* get the ag for the iag */
>   	agstart = le64_to_cpu(iagp->agstart);
> +	agno = BLKTOAG(agstart, JFS_SBI(ip->i_sb));
>   
>   	release_metapage(mp);
> +	if (agno >= MAXAG || agno < 0)
> +		return -EIO;
>   
>   	rel_inode = (ino & (INOSPERPAGE - 1));
>   	pageno = blkno >> sbi->l2nbperpage;
diff mbox series

Patch

diff --git a/fs/jfs/jfs_imap.c b/fs/jfs/jfs_imap.c
index 2ec35889ad24..0aac083bc0db 100644
--- a/fs/jfs/jfs_imap.c
+++ b/fs/jfs/jfs_imap.c
@@ -881,6 +881,11 @@  int diFree(struct inode *ip)
 	 */
 	agno = BLKTOAG(JFS_IP(ip)->agstart, JFS_SBI(ip->i_sb));
 
+	if(agno >= MAXAG || agno < 0){
+		jfs_error(ip->i_sb, "invalid array index (0 <= agno < MAXAG), agno = %d\n", agno);
+		return -ENOMEM;
+	}
+
 	/* Lock the AG specific inode map information
 	 */
 	AG_LOCK(imap, agno);