Message-ID: <2025021902-overspend-buckwheat-e5c3@gregkh>
Date: Wed, 19 Feb 2025 06:10:42 +0100
From: Greg KH <gregkh@...uxfoundation.org>
To: 2025021350-geometry-appear-9d84@...gkh.smtp.subspace.kernel.org
Cc: shaggy@...nel.org, zhaomengmeng@...inos.cn, llfamsec@...il.com,
	ancowi69@...il.com, jfs-discussion@...ts.sourceforge.net,
	linux-kernel@...r.kernel.org,
	syzbot+4e6e7e4279d046613bc5@...kaller.appspotmail.com,
	stable@...r.kernel.org
Subject: Re: [PATCH] jfs: fix slab-out-of-bounds read in ea_get()

On Wed, Feb 19, 2025 at 12:39:47AM +0000, Qasim Ijaz wrote:
> On Thu, Feb 13, 2025 at 11:07:07AM +0100, Greg KH wrote:
> > On Thu, Feb 13, 2025 at 12:20:25AM +0000, Qasim Ijaz wrote:
> > > During the "size_check" label in ea_get(), the code checks if the extended 
> > > attribute list (xattr) size matches ea_size. If not, it logs 
> > > "ea_get: invalid extended attribute" and calls print_hex_dump().
> > > 
> > > Here, EALIST_SIZE(ea_buf->xattr) returns 4110417968, which exceeds 
> > > INT_MAX (2,147,483,647). Then ea_size is clamped:
> > > 
> > > 	int size = clamp_t(int, ea_size, 0, EALIST_SIZE(ea_buf->xattr));
> > > 
> > > Although clamp_t aims to bound ea_size between 0 and 4110417968, the upper 
> > > limit is treated as an int, causing an overflow above 2^31 - 1. This leads 
> > > "size" to wrap around and become negative (-184549328).
> > > 
> > > The "size" is then passed to print_hex_dump() (called "len" in 
> > > print_hex_dump()), it is passed as type size_t (an unsigned 
> > > type), this is then stored inside a variable called 
> > > "int remaining", which is then assigned to "int linelen" which 
> > > is then passed to hex_dump_to_buffer(). In print_hex_dump() 
> > > the for loop, iterates through 0 to len-1, where len is 
> > > 18446744073525002176, calling hex_dump_to_buffer() 
> > > on each iteration:
> > > 
> > > 	for (i = 0; i < len; i += rowsize) {
> > > 		linelen = min(remaining, rowsize);
> > > 		remaining -= rowsize;
> > > 
> > > 		hex_dump_to_buffer(ptr + i, linelen, rowsize, groupsize,
> > > 				   linebuf, sizeof(linebuf), ascii);
> > > 	
> > > 		...
> > > 	}
> > > 	
> > > The expected stopping condition (i < len) is effectively broken 
> > > since len is corrupted and very large. This eventually leads to 
> > > the "ptr+i" being passed to hex_dump_to_buffer() to get closer 
> > > to the end of the actual bounds of "ptr", eventually an out of 
> > > bounds access is done in hex_dump_to_buffer() in the following 
> > > for loop:
> > > 
> > > 	for (j = 0; j < len; j++) {
> > > 			if (linebuflen < lx + 2)
> > > 				goto overflow2;
> > > 			ch = ptr[j];
> > > 		...
> > > 	}
> > > 
> > > To fix this we should validate "EALIST_SIZE(ea_buf->xattr)" 
> > > before it is utilised.
> > > 
> > > Reported-by: syzbot <syzbot+4e6e7e4279d046613bc5@...kaller.appspotmail.com>
> > > Tested-by: syzbot <syzbot+4e6e7e4279d046613bc5@...kaller.appspotmail.com>
> > > Closes: https://syzkaller.appspot.com/bug?extid=4e6e7e4279d046613bc5
> > > Fixes: d9f9d96136cb ("jfs: xattr: check invalid xattr size more strictly")
> > > Signed-off-by: Qasim Ijaz <qasdev00@...il.com>
> > > ---
> > >  fs/jfs/xattr.c | 15 ++++++++++-----
> > >  1 file changed, 10 insertions(+), 5 deletions(-)
> > > 
> > > diff --git a/fs/jfs/xattr.c b/fs/jfs/xattr.c
> > > index 24afbae87225..7575c51cce9b 100644
> > > --- a/fs/jfs/xattr.c
> > > +++ b/fs/jfs/xattr.c
> > > @@ -559,11 +555,16 @@ static int ea_get(struct inode *inode, struct ea_buffer *ea_buf, int min_size)
> > >  
> > >        size_check:
> > >  	if (EALIST_SIZE(ea_buf->xattr) != ea_size) {
> > > -		int size = clamp_t(int, ea_size, 0, EALIST_SIZE(ea_buf->xattr));
> > > -
> > > -		printk(KERN_ERR "ea_get: invalid extended attribute\n");
> > > -		print_hex_dump(KERN_ERR, "", DUMP_PREFIX_ADDRESS, 16, 1,
> > > -				     ea_buf->xattr, size, 1);
> > > +		if (unlikely(EALIST_SIZE(ea_buf->xattr) > INT_MAX)) {
> > > +			printk(KERN_ERR "ea_get: extended attribute size too large: %u > INT_MAX\n",
> > > +			       EALIST_SIZE(ea_buf->xattr));
> > > +		} else {
> > > +			int size = clamp_t(int, ea_size, 0, EALIST_SIZE(ea_buf->xattr));
> > > +
> > > +			printk(KERN_ERR "ea_get: invalid extended attribute\n");
> > > +			print_hex_dump(KERN_ERR, "", DUMP_PREFIX_ADDRESS, 16, 1,
> > > +				       ea_buf->xattr, size, 1);
> > > +		}
> > >  		ea_release(inode, ea_buf);
> > >  		rc = -EIO;
> > >  		goto clean_up;
> > > -- 
> > > 2.39.5
> > > 
> > 
> > Hi,
> > 
> > This is the friendly patch-bot of Greg Kroah-Hartman.  You have sent him
> > a patch that has triggered this response.  He used to manually respond
> > to these common problems, but in order to save his sanity (he kept
> > writing the same thing over and over, yet to different people), I was
> > created.  Hopefully you will not take offence and will fix the problem
> > in your patch and resubmit it so that it can be accepted into the Linux
> > kernel tree.
> > 
> > You are receiving this message because of the following common error(s)
> > as indicated below:
> > 
> > - You have marked a patch with a "Fixes:" tag for a commit that is in an
> >   older released kernel, yet you do not have a cc: stable line in the
> >   signed-off-by area at all, which means that the patch will not be
> >   applied to any older kernel releases.  To properly fix this, please
> >   follow the documented rules in the
> >   Documentation/process/stable-kernel-rules.rst file for how to resolve
> >   this.
> > 
> > If you wish to discuss this problem further, or you have questions about
> > how to resolve this issue, please feel free to respond to this email and
> > Greg will reply once he has dug out from the pending patches received
> > from other developers.
> >
> Hi Greg,
> 
> Just following up on this patch. I’ve sent v2 with the added CC stable tag. Here’s the link:
> https://lore.kernel.org/all/20250213210553.28613-1-qasdev00@gmail.com/
> 
> Let me know if any further changes are needed.

It's been less than one week, for a filesystem that is not actively
maintained and no one should be using anymore, so please be patient.

thanks,

greg k-h

Powered by blists - more mailing lists