jfs: fix slab-out-of-bounds read in ea_get()
Subsystems (2)
PATCH v2 1/1 jfs: fix slab-out-of-bounds read in ea_get()
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.
---
v2:
- Added Cc stable tag
fs/jfs/xattr.c | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-)
fs/jfs/xattr.c +10 -5
@@ -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;
Signature · 1 line
2.39.5
Quoted 46 lines · Qasim Ijaz
During the "
size_check" label inea_get(), the code checks if the extended attribute list (xattr) size matchesea_size. If not, it logs "ea_get: invalid extended attribute" and callsprint_hex_dump().Here,
EALIST_SIZE(ea_buf->xattr)returns 4110417968, which exceedsINT_MAX(2,147,483,647). Thenea_sizeis clamped:int size = clamp_t(int, ea_size, 0, EALIST_SIZE(ea_buf->xattr));Although
clamp_taims to boundea_sizebetween 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" inprint_hex_dump()), it is passed as typesize_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 tohex_dump_to_buffer(). Inprint_hex_dump()the for loop, iterates through 0 to len-1, where len is 18446744073525002176, callinghex_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 inhex_dump_to_buffer()in the following for loop:for (j = 0; j < len; j++) { if (linebuflen < lx + 2) goto overflow2; ch = ptr[j]; ... }
Quoted 2 lines
To fix this we should validate "
EALIST_SIZE(ea_buf->xattr)" before it is utilised.
Agreed. The patch looks good. I'll add it to jfs-next
Shaggy
Quoted 40 lines
Reported-by syzbot <[email protected]>Tested-by syzbot <[email protected]>Fixes d9f9d96136cb ("jfs: xattr: check invalid xattr size more strictly")Signed-off-by Qasim Ijaz <[email protected]>---
v2:
- Added Cc stable tagfs/jfs/xattr.c | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-)fs/jfs/xattr.c +10
-5@@ -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;