Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [High] tracing/user_events: u32 struct size wrap in user_event_parse_field() --
commit 3b2945a002d22d5a8a1ae3cae8f4c9ec6c460511 Author: Yifan Zhang <[email protected]> tracing/user_events: reject overflowing field offsets This patch adds an overflow check in user_event_parse_field() to reject field offsets that overflow a u32 or exceed INT_MAX when registering a user event. > diff --git a/kernel/trace/trace_events_user.c > b/kernel/trace/trace_events_user.c > index 85938b45b7618..88d384c9e101f 100644 > --- a/kernel/trace/trace_events_user.c > +++ b/kernel/trace/trace_events_user.c > @@ -1207,6 +1207,7 @@ static int user_event_parse_field(char *field, struct > user_event *user, > { > char *part, *type, *name; > u32 depth = 0, saved_offset = *offset; > + u32 new_offset; > int len, size = -EINVAL; > bool is_struct = false; > > @@ -1287,7 +1288,11 @@ static int user_event_parse_field(char *field, struct > user_event *user, > if (size < 0) > return size; > > - *offset = saved_offset + size; > + if (check_add_overflow(saved_offset, (u32)size, &new_offset) || > + new_offset > INT_MAX) > + return -E2BIG; [Severity: High] What happens when a user supplies a struct field size between INT_MAX + 1 and U32_MAX? Earlier in user_event_parse_field(), the struct size is parsed into the signed int variable size via kstrtou32(): case FIELD_DEPTH_SIZE: if (!is_struct) return -EINVAL; if (kstrtou32(part, 10, &size)) return -EINVAL; break; When part is in [2147483648, 4294967295], kstrtou32() succeeds and writes a value into size that is negative when interpreted as a signed int. The preceding check: if (size < 0) return size; then returns that user-controlled negative integer directly instead of reaching the new overflow check. When reached through user_events_ioctl() -> user_events_ioctl_reg() -> user_event_parse_cmd() -> user_event_parse() -> user_event_parse_fields() -> user_event_parse_field(), or through create_dyn_event() -> user_event_create(), could this return arbitrary negative values (including kernel-internal restart codes like -ERESTARTSYS, values outside [-MAX_ERRNO, -1], or -ECANCELED) to the caller? Also, since both saved_offset and size are bounded to INT_MAX after the size < 0 check, can check_add_overflow() ever trigger here? -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=2
