lkml.org 
[lkml]   [2017]   [May]   [22]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [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.

\
 
 \ /
  Last update: 2017-05-23 00:41    [W:0.088 / U:0.168 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site