linux-kernel - Re: [PATCH 01/14] sysctl: provide callback for write into ctl

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] [day] [month] [year] [list]

Date:	Sun, 29 Apr 2012 19:52:37 -0700
From:	ebiederm@...ssion.com (Eric W. Biederman)
To:	Steven Rostedt <rostedt@...dmis.org>
Cc:	Sasha Levin <levinsasha928@...il.com>, viro@...iv.linux.org.uk,
	fweisbec@...il.com, mingo@...hat.com, a.p.zijlstra@...llo.nl,
	paulus@...ba.org, acme@...stprotocols.net,
	james.l.morris@...cle.com, akpm@...ux-foundation.org,
	tglx@...utronix.de, linux-fsdevel@...r.kernel.org,
	linux-kernel@...r.kernel.org, linux-mm@...ck.org,
	linux-security-module@...r.kernel.org,
	Eric Paris <eparis@...isplace.org>
Subject: Re: [PATCH 01/14] sysctl: provide callback for write into ctl_table entry

Steven Rostedt <rostedt@...dmis.org> writes:

> On Sun, 2012-04-29 at 16:14 +0200, Sasha Levin wrote:
>
>> A fix for that could be having the sysctl modifying a different var,
>> and having ftrace_enabled from that under a lock, but I'm not sure if
>> it's worth the work for the cleanup.
>
> That was my original plan, but it seemed too much of a hassle than it
> was worth, as I needed to make sure the mirrored variable was in sync
> with ftrace_enabled, otherwise it could be confusing when ftrace was not
> working but sysctl showed ftrace set to 1.

I don't see the problem you are trying to solve with your patches.

What I do see is you have ignored one of the biggest problem with the
current sysctl interface in that it is not easy to plug in your own code
in the cases you need to before an update is made. (Locks permission
checks, etc).

You have also bloated struct ctl_table for no apparent reason.

This current crop of patches was just sloppy.  You showed a poor
choice of function names and did not preserve necessary invariants
when changing the code.  It looks like you exchanged something that
was a bit ugly for something that straight out encourages broken
behavior. 

So I respectfully suggest you go back to the drawing board and figure
out a solution that makes this class of function much easier to write
in a bug free manner.

Eric
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/