Commit e7d316a02f6838 ("sysctl: handle error writing UINT_MAX to u32 fields") added proc_douintvec() to start help adding support for unsigned int, this however was only half the work needed, all these issues are present with the current implementation: o Printing the values shows a negative value, this happens since do_proc_dointvec() and this uses proc_put_long() o We can easily wrap around the int values: UINT_MAX is 4294967295, if we echo in 4294967295 + 1 we end up with 0, using 4294967295 + 2 we end up with 1. o We echo negative values in and they are accepted Fix all these issues by adding our own do_proc_douintvec(). Cc: Subash Abhinov Kasiviswanathan <subashab@xxxxxxxxxxxxxx> Cc: Heinrich Schuchardt <xypron.glpk@xxxxxx> Cc: Kees Cook <keescook@xxxxxxxxxxxx> Cc: "David S. Miller" <davem@xxxxxxxxxxxxx> Cc: Ingo Molnar <mingo@xxxxxxxxxx> Cc: Andrew Morton <akpm@xxxxxxxxxxxxxxxxxxxx> Cc: Linus Torvalds <torvalds@xxxxxxxxxxxxxxxxxxxx> Cc: Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx> Fixes: e7d316a02f68 ("sysctl: handle error writing UINT_MAX to u32 fields") Signed-off-by: Luis R. Rodriguez <mcgrof@xxxxxxxxxx> --- I split this off as its own atomic fix from a larger RFC series [0]. I've only provided the fix here, and split off further functionality into a separate patch for the future. Although this is a fix I don't think its super critical, and specially due to its size do not think it can be stable material. I do have proc_douintvec_minmax() but since we have no users for it it can wait until I add something that makes use of it. If someone needs it now though please let me know. Likewise adding proc_douintvec_minmax_sysadmin() is very trivial but I have no immediate users for it so it can wait even longer. [0] https://lkml.kernel.org/r/20161208184801.1689-1-mcgrof@xxxxxxxxxx kernel/sysctl.c | 121 +++++++++++++++++++++++++++++++++++++++++++++++++++++--- 1 file changed, 115 insertions(+), 6 deletions(-) diff --git a/kernel/sysctl.c b/kernel/sysctl.c index 8dbaec0e4f7f..118341d3a139 100644 --- a/kernel/sysctl.c +++ b/kernel/sysctl.c @@ -2125,12 +2125,12 @@ static int do_proc_dointvec_conv(bool *negp, unsigned long *lvalp, return 0; } -static int do_proc_douintvec_conv(bool *negp, unsigned long *lvalp, - int *valp, - int write, void *data) +static int do_proc_douintvec_conv(unsigned long *lvalp, + unsigned int *valp, + int write, void *data) { if (write) { - if (*negp) + if (*lvalp > (unsigned long) UINT_MAX) return -EINVAL; *valp = *lvalp; } else { @@ -2243,6 +2243,115 @@ static int do_proc_dointvec(struct ctl_table *table, int write, buffer, lenp, ppos, conv, data); } +static int __do_proc_douintvec(void *tbl_data, struct ctl_table *table, + int write, void __user *buffer, + size_t *lenp, loff_t *ppos, + int (*conv)(unsigned long *lvalp, + unsigned int *valp, + int write, void *data), + void *data) +{ + unsigned int *i, vleft; + bool first = true; + int err = 0; + size_t left; + char *kbuf = NULL, *p; + + if (!tbl_data || !table->maxlen || !*lenp || (*ppos && !write)) { + *lenp = 0; + return 0; + } + + i = (unsigned int *) tbl_data; + vleft = table->maxlen / sizeof(*i); + left = *lenp; + + if (!conv) + conv = do_proc_douintvec_conv; + + if (write) { + if (*ppos) { + switch (sysctl_writes_strict) { + case SYSCTL_WRITES_STRICT: + goto out; + case SYSCTL_WRITES_WARN: + warn_sysctl_write(table); + break; + default: + break; + } + } + + if (left > PAGE_SIZE - 1) + left = PAGE_SIZE - 1; + p = kbuf = memdup_user_nul(buffer, left); + if (IS_ERR(kbuf)) + return PTR_ERR(kbuf); + } + + for (; left && vleft--; i++, first=false) { + unsigned long lval; + bool neg; + + if (write) { + left -= proc_skip_spaces(&p); + + if (!left) + break; + err = proc_get_long(&p, &left, &lval, &neg, + proc_wspace_sep, + sizeof(proc_wspace_sep), NULL); + if (neg) { + err = -EINVAL; + break; + } + if (err) + break; + if (conv(&lval, i, 1, data)) { + err = -EINVAL; + break; + } + } else { + if (conv(&lval, i, 0, data)) { + err = -EINVAL; + break; + } + if (!first) + err = proc_put_char(&buffer, &left, '\t'); + if (err) + break; + err = proc_put_long(&buffer, &left, lval, false); + if (err) + break; + } + } + + if (!write && !first && left && !err) + err = proc_put_char(&buffer, &left, '\n'); + if (write && !err && left) + left -= proc_skip_spaces(&p); + if (write) { + kfree(kbuf); + if (first) + return err ? : -EINVAL; + } + *lenp -= left; +out: + *ppos += *lenp; + return err; +} + +static int do_proc_douintvec(struct ctl_table *table, int write, + void __user *buffer, size_t *lenp, loff_t *ppos, + int (*conv)(unsigned long *lvalp, + unsigned int *valp, + int write, void *data), + void *data) +{ + return __do_proc_douintvec(table->data, table, write, + buffer, lenp, ppos, conv, data); +} + /** * proc_dointvec - read a vector of integers * @table: the sysctl table @@ -2278,8 +2387,8 @@ int proc_dointvec(struct ctl_table *table, int write, int proc_douintvec(struct ctl_table *table, int write, void __user *buffer, size_t *lenp, loff_t *ppos) { - return do_proc_dointvec(table, write, buffer, lenp, ppos, - do_proc_douintvec_conv, NULL); + return do_proc_douintvec(table, write, buffer, lenp, ppos, + do_proc_douintvec_conv, NULL); } /* -- 2.11.0 -- To unsubscribe from this list: send the line "unsubscribe linux-kselftest" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html