Messages in this thread | | | Date | Mon, 22 May 2017 15:40:55 -0700 | From | Andrew Morton <> | Subject | Re: [PATCH v3 1/5] sysctl: fix lax sysctl_check_table() sanity check |
| |
On Thu, 18 May 2017 20:35:50 -0700 "Luis R. Rodriguez" <mcgrof@kernel.org> wrote:
> Commit 7c60c48f58a7 ("sysctl: Improve the sysctl sanity checks") > improved sanity checks considerbly, however the enhancements on > sysctl_check_table() meant adding a functional change so that > only the last table entry's sanity error is propagated. It also > changed the way errors were propagated so that each new check > reset the err value, this means only last sanity check computed > is used for an error. This has been in the kernel since v3.4 days. > > Fix this by carrying on errors from previous checks and iterations > as we traverse the table and ensuring we keep any error from previous > checks. We keep iterating on the table even if an error is found so > we can complain for all errors found in one shot. This works as > -EINVAL is always returned on error anyway, and the check for error > is any non-zero value. > > ... > > --- a/fs/proc/proc_sysctl.c > +++ b/fs/proc/proc_sysctl.c > @@ -1066,7 +1066,7 @@ static int sysctl_check_table(const char *path, struct ctl_table *table) > int err = 0; > for (; table->procname; table++) { > if (table->child) > - err = sysctl_err(path, table, "Not a file"); > + err |= sysctl_err(path, table, "Not a file"); > > if ((table->proc_handler == proc_dostring) || > (table->proc_handler == proc_dointvec) || > @@ -1078,15 +1078,15 @@ static int sysctl_check_table(const char *path, struct ctl_table *table) > (table->proc_handler == proc_doulongvec_minmax) || > (table->proc_handler == proc_doulongvec_ms_jiffies_minmax)) { > if (!table->data) > - err = sysctl_err(path, table, "No data"); > + err |= sysctl_err(path, table, "No data"); > if (!table->maxlen) > - err = sysctl_err(path, table, "No maxlen"); > + err |= sysctl_err(path, table, "No maxlen"); > } > if (!table->proc_handler) > - err = sysctl_err(path, table, "No proc_handler"); > + err |= sysctl_err(path, table, "No proc_handler"); > > if ((table->mode & (S_IRUGO|S_IWUGO)) != table->mode) > - err = sysctl_err(path, table, "bogus .mode 0%o", > + err |= sysctl_err(path, table, "bogus .mode 0%o", > table->mode); > } > return err;
glumpf. I'm not a fan of this err|=foo() trick - if foo() returns different errnos we can a mangled result. This patch assumes that syscal_err() will only ever return a single errno.
I guess we're safe enough in this case but still... ugh.
| |