Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [High] Information Leak / Uninitialized Memory Disclosure in param_array_get() --
commit 4bb10449d07e3a1b177001d61d53053c25dd4767 Author: Kees Cook <[email protected]> params: Convert generic kernel_param_ops .get helpers to seq_buf This commit converts the generic kernel parameter .get helpers in kernel/params.c to use the seq_buf signature instead of char pointers. It allows param_array_get to write element output directly into the parent seq_buf when supported, updating various dependent callers to match the new signature. > diff --git a/kernel/params.c b/kernel/params.c > index 4eda2d23ddf28..25f0c8d5d19f3 100644 > --- a/kernel/params.c > +++ b/kernel/params.c [ ... ] > @@ -453,36 +457,46 @@ static int param_array_set(const char *val, const > struct kernel_param *kp) [ ... ] > -static int param_array_get(char *buffer, const struct kernel_param *kp) > +static int param_array_get(struct seq_buf *s, const struct kernel_param *kp) > { [ ... ] > - for (i = off = 0; i < (arr->num ? *arr->num : arr->max); i++) { > + for (i = 0; i < (arr->num ? *arr->num : arr->max); i++) { > + size_t before = s->len; > > p.arg = arr->elem + arr->elemsize * i; > check_kparam_locked(p.mod); > - ret = arr->ops->get_str(elem_buf, &p); > - if (ret < 0) > - goto out; > - ret = min(ret, (int)(PAGE_SIZE - 1 - off)); > - if (!ret) > + > + if (arr->ops->get) { > + ret = arr->ops->get(s, &p); > + if (ret < 0) > + goto out; > + } else { [ ... ] > + ret = arr->ops->get_str(elem_buf, &p); > + if (ret < 0) > + goto out; > + seq_buf_putmem(s, elem_buf, ret); > + } > + > + /* Nothing got written (e.g. overflow) — stop. */ > + if (s->len == before) > break; [Severity: High] When a seq_buf overflows, it calls seq_buf_set_overflow() which sets s->len = s->size. Because of this, does s->len actually equal before when an overflow occurs? If s->len becomes s->size, the condition s->len == before will evaluate to false, causing the loop to continue for one more iteration rather than breaking. On the next iteration, before will be s->size, the write will fail again, and the loop will finally break. Because the overflow mechanism sets the length to the buffer size without actually writing data into the remaining space, could this leak uninitialized kernel heap memory to userspace when sysfs returns the buffer? > + > + /* Replace the previous element's trailing newline with a > comma. */ > - if (i) > - buffer[off - 1] = ','; > - memcpy(buffer + off, elem_buf, ret); > - off += ret; > - if (off == PAGE_SIZE - 1) > - break; > + if (i && s->buffer[before - 1] == '\n') > + s->buffer[before - 1] = ','; > } > - buffer[off] = '\0'; > - ret = off; > + ret = 0; > out: > kfree(elem_buf); > return ret; > } -- Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=8
