linux-kernel - [PATCH v3 1/5] sysctl: fix lax sysctl_check

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Thu, 18 May 2017 20:35:50 -0700
From:   "Luis R. Rodriguez" <mcgrof@...nel.org>
To:     viro@...iv.linux.org.uk, akpm@...ux-foundation.org,
        ebiederm@...ssion.com, keescook@...omium.org, acme@...hat.com,
        mingo@...nel.org, mgorman@...e.de, subashab@...eaurora.org
Cc:     jeyu@...hat.com, rusty@...tcorp.com.au, swhiteho@...hat.com,
        deepa.kernel@...il.com, matt@...eblueprint.co.uk,
        adobriyan@...il.com, bp@...e.de, zlpnobody@...il.com,
        dmitry.torokhov@...il.com, shuah@...nel.org,
        torvalds@...ux-foundation.org, linux@...ck-us.net,
        linux-kernel@...r.kernel.org,
        "Luis R. Rodriguez" <mcgrof@...nel.org>
Subject: [PATCH v3 1/5] sysctl: fix lax sysctl_check_table() sanity check

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.

Fixes: 7c60c48f58a7 ("sysctl: Improve the sysctl sanity checks")
Signed-off-by: Luis R. Rodriguez <mcgrof@...nel.org>
---
 fs/proc/proc_sysctl.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/fs/proc/proc_sysctl.c b/fs/proc/proc_sysctl.c
index 67985a7233c2..32c9c5630507 100644
--- 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;
-- 
2.11.0