On 1/7/26 08:57, Chao Yu via Linux-f2fs-devel wrote: > On 1/6/2026 7:56 PM, Yongpeng Yang wrote: >> From: Yongpeng Yang <[email protected]> >> >> Some f2fs sysfs attributes suffer from out-of-bounds memory access and >> incorrect handling of integer values whose size is not 4 bytes. >> >> For example: >> vm:~# echo 65537 > /sys/fs/f2fs/vde/carve_out >> vm:~# cat /sys/fs/f2fs/vde/carve_out >> 65537 >> vm:~# echo 4294967297 > /sys/fs/f2fs/vde/atgc_age_threshold >> vm:~# cat /sys/fs/f2fs/vde/atgc_age_threshold >> 1 >> >> carve_out maps to {struct f2fs_sb_info}->carve_out, which is a 8-bit >> integer. However, the sysfs interface allows setting it to a value >> larger than 255, resulting in an out-of-range update. >> >> atgc_age_threshold maps to {struct atgc_management}->age_threshold, >> which is a 64-bit integer, but its sysfs interface cannot correctly set >> values larger than UINT_MAX. >> >> The root causes are: >> 1. __sbi_store() treats all default values as unsigned int, which >> prevents updating integers larger than 4 bytes and causes out-of-bounds >> writes for integers smaller than 4 bytes. >> >> 2. f2fs_sbi_show() also assumes all default values are unsigned int, >> leading to out-of-bounds reads and incorrect access to integers larger >> than 4 bytes. >> >> This patch introduces {struct f2fs_attr}->size to record the actual size >> of the integer associated with each sysfs attribute. With this >> information, sysfs read and write operations can correctly access and >> update values according to their real data size, avoiding memory >> corruption and truncation. >> >> Fixes: b59d0bae6ca3("f2fs: add sysfs support for controlling the >> gc_thread") > > ./scripts/checkpatch.pl triggers warning as below: > > WARNING: Please use correct Fixes: style 'Fixes: <12+ chars of sha1> > ("<title line>")' - ie: 'Fixes: b59d0bae6ca3 ("f2fs: add sysfs support > for controlling the gc_thread")' > #40: > Fixes: b59d0bae6ca3("f2fs: add sysfs support for controlling the > gc_thread")
Sorry, my mistake. I'll fix this in v3 patch. Thanks Yongpeng, > > Otherwise, the patch looks good to me. > > Thanks, > >> Cc: [email protected] >> Signed-off-by: Jinbao Liu <[email protected]> >> Signed-off-by: Yongpeng Yang <[email protected]> >> --- >> v2: >> - Replace gc_pin_file_thresh example with carve_out in the commit >> message. >> --- >> fs/f2fs/sysfs.c | 60 ++++++++++++++++++++++++++++++++++++++++++------- >> 1 file changed, 52 insertions(+), 8 deletions(-) >> >> diff --git a/fs/f2fs/sysfs.c b/fs/f2fs/sysfs.c >> index c42f4f979d13..e6a98ddd73b3 100644 >> --- a/fs/f2fs/sysfs.c >> +++ b/fs/f2fs/sysfs.c >> @@ -58,6 +58,7 @@ struct f2fs_attr { >> const char *buf, size_t len); >> int struct_type; >> int offset; >> + int size; >> int id; >> }; >> @@ -344,11 +345,30 @@ static ssize_t main_blkaddr_show(struct >> f2fs_attr *a, >> (unsigned long long)MAIN_BLKADDR(sbi)); >> } >> +static ssize_t __sbi_show_value(struct f2fs_attr *a, >> + struct f2fs_sb_info *sbi, char *buf, >> + unsigned char *value) >> +{ >> + switch (a->size) { >> + case 1: >> + return sysfs_emit(buf, "%u\n", *(u8 *)value); >> + case 2: >> + return sysfs_emit(buf, "%u\n", *(u16 *)value); >> + case 4: >> + return sysfs_emit(buf, "%u\n", *(u32 *)value); >> + case 8: >> + return sysfs_emit(buf, "%llu\n", *(u64 *)value); >> + default: >> + f2fs_bug_on(sbi, 1); >> + return sysfs_emit(buf, >> + "show sysfs node value with wrong type\n"); >> + } >> +} >> + >> static ssize_t f2fs_sbi_show(struct f2fs_attr *a, >> struct f2fs_sb_info *sbi, char *buf) >> { >> unsigned char *ptr = NULL; >> - unsigned int *ui; >> ptr = __struct_ptr(sbi, a->struct_type); >> if (!ptr) >> @@ -428,9 +448,30 @@ static ssize_t f2fs_sbi_show(struct f2fs_attr *a, >> atomic_read(&sbi->cp_call_count[BACKGROUND])); >> #endif >> - ui = (unsigned int *)(ptr + a->offset); >> + return __sbi_show_value(a, sbi, buf, ptr + a->offset); >> +} >> - return sysfs_emit(buf, "%u\n", *ui); >> +static void __sbi_store_value(struct f2fs_attr *a, >> + struct f2fs_sb_info *sbi, >> + unsigned char *ui, unsigned long value) >> +{ >> + switch (a->size) { >> + case 1: >> + *(u8 *)ui = value; >> + break; >> + case 2: >> + *(u16 *)ui = value; >> + break; >> + case 4: >> + *(u32 *)ui = value; >> + break; >> + case 8: >> + *(u64 *)ui = value; >> + break; >> + default: >> + f2fs_bug_on(sbi, 1); >> + f2fs_err(sbi, "store sysfs node value with wrong type"); >> + } >> } >> static ssize_t __sbi_store(struct f2fs_attr *a, >> @@ -906,7 +947,7 @@ static ssize_t __sbi_store(struct f2fs_attr *a, >> return count; >> } >> - *ui = (unsigned int)t; >> + __sbi_store_value(a, sbi, ptr + a->offset, t); >> return count; >> } >> @@ -1053,24 +1094,27 @@ static struct f2fs_attr f2fs_attr_sb_##_name = >> { \ >> .id = F2FS_FEATURE_##_feat, \ >> } >> -#define F2FS_ATTR_OFFSET(_struct_type, _name, _mode, _show, _store, >> _offset) \ >> +#define F2FS_ATTR_OFFSET(_struct_type, _name, _mode, _show, _store, >> _offset, _size) \ >> static struct f2fs_attr f2fs_attr_##_name = { \ >> .attr = {.name = __stringify(_name), .mode = _mode }, \ >> .show = _show, \ >> .store = _store, \ >> .struct_type = _struct_type, \ >> - .offset = _offset \ >> + .offset = _offset, \ >> + .size = _size \ >> } >> #define F2FS_RO_ATTR(struct_type, struct_name, name, elname) \ >> F2FS_ATTR_OFFSET(struct_type, name, 0444, \ >> f2fs_sbi_show, NULL, \ >> - offsetof(struct struct_name, elname)) >> + offsetof(struct struct_name, elname), \ >> + sizeof_field(struct struct_name, elname)) >> #define F2FS_RW_ATTR(struct_type, struct_name, name, elname) \ >> F2FS_ATTR_OFFSET(struct_type, name, 0644, \ >> f2fs_sbi_show, f2fs_sbi_store, \ >> - offsetof(struct struct_name, elname)) >> + offsetof(struct struct_name, elname), \ >> + sizeof_field(struct struct_name, elname)) >> #define F2FS_GENERAL_RO_ATTR(name) \ >> static struct f2fs_attr f2fs_attr_##name = __ATTR(name, 0444, >> name##_show, NULL) _______________________________________________ Linux-f2fs-devel mailing list [email protected] https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
