Messages in this thread Patch in this message | | | From | Xia Fukun <> | Subject | [PATCH v4] kobject: Fix global-out-of-bounds in kobject_action_type() | Date | Fri, 12 May 2023 18:30:29 +0800 |
| |
The following c language code can trigger KASAN's global variable out-of-bounds access error in kobject_action_type():
int main() { int fd; char *filename = "/sys/block/ram12/uevent"; char str[86] = "offline"; int len = 86;
fd = open(filename, O_WRONLY); if (fd == -1) { printf("open"); exit(1); }
if (write(fd, str, len) == -1) { printf("write"); exit(1); }
close(fd); return 0; }
Function kobject_action_type() receives the input parameters buf and count, where count is the length of the string buf.
In the use case we provided, count is 86, the count_first is 85. Buf points to a string with a length of 86, and its first seven characters are "offline". In line 87 of the code, kobject_actions[action] is the string "offline" with the length of 7,an out-of-boundary access will appear:
kobject_actions[action][85].
Use sysfs_match_string() to replace the fragile and convoluted loop. This function is well-tested for parsing sysfs inputs. Moreover, this modification will not cause any functional changes.
Fixes: f36776fafbaa ("kobject: support passing in variables for synthetic uevents") Signed-off-by: Xia Fukun <xiafukun@huawei.com> --- v3 -> v4: - refactor the function to be more obviously correct and readable
v2 -> v3: - only declare that it is the latest version of the patch, no change
v1 -> v2: - modify the matching logic
lib/kobject_uevent.c | 27 +++++++++++++++------------ 1 file changed, 15 insertions(+), 12 deletions(-)
diff --git a/lib/kobject_uevent.c b/lib/kobject_uevent.c index 7c44b7ae4c5c..59f835f5f870 100644 --- a/lib/kobject_uevent.c +++ b/lib/kobject_uevent.c @@ -66,7 +66,7 @@ static int kobject_action_type(const char *buf, size_t count, enum kobject_action action; size_t count_first; const char *args_start; - int ret = -EINVAL; + int i, ret = -EINVAL; if (count && (buf[count-1] == '\n' || buf[count-1] == '\0')) count--; @@ -81,17 +81,20 @@ static int kobject_action_type(const char *buf, size_t count, } else count_first = count; - for (action = 0; action < ARRAY_SIZE(kobject_actions); action++) { - if (strncmp(kobject_actions[action], buf, count_first) != 0) - continue; - if (kobject_actions[action][count_first] != '\0') - continue; - if (args) - *args = args_start; - *type = action; - ret = 0; - break; - } + /* Use sysfs_match_string() to replace the fragile and convoluted loop */ + i = sysfs_match_string(kobject_actions, buf); + + if (i < 0) + return ret; + + action = kobject_action(i); + + if (args) + *args = args_start; + + *type = action; + ret = 0; + out: return ret; } -- 2.17.1
| |