From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1758930AbdEVWk6 (ORCPT ); Mon, 22 May 2017 18:40:58 -0400 Received: from mail.linuxfoundation.org ([140.211.169.12]:49310 "EHLO mail.linuxfoundation.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751623AbdEVWk5 (ORCPT ); Mon, 22 May 2017 18:40:57 -0400 Date: Mon, 22 May 2017 15:40:55 -0700 From: Andrew Morton To: "Luis R. Rodriguez" Cc: viro@zeniv.linux.org.uk, ebiederm@xmission.com, keescook@chromium.org, acme@redhat.com, mingo@kernel.org, mgorman@suse.de, subashab@codeaurora.org, jeyu@redhat.com, rusty@rustcorp.com.au, swhiteho@redhat.com, deepa.kernel@gmail.com, matt@codeblueprint.co.uk, adobriyan@gmail.com, bp@suse.de, zlpnobody@gmail.com, dmitry.torokhov@gmail.com, shuah@kernel.org, torvalds@linux-foundation.org, linux@roeck-us.net, linux-kernel@vger.kernel.org Subject: Re: [PATCH v3 1/5] sysctl: fix lax sysctl_check_table() sanity check Message-Id: <20170522154055.0abb1ef6e17bb9d3ac75cb73@linux-foundation.org> In-Reply-To: <20170519033554.18592-2-mcgrof@kernel.org> References: <20170211003614.6579-1-mcgrof@kernel.org> <20170519033554.18592-1-mcgrof@kernel.org> <20170519033554.18592-2-mcgrof@kernel.org> X-Mailer: Sylpheed 3.4.1 (GTK+ 2.24.23; x86_64-pc-linux-gnu) Mime-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Thu, 18 May 2017 20:35:50 -0700 "Luis R. Rodriguez" 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.