linux-kernel - Re: [PATCH] kernel/sysctl.c: If "count" including the terminating byte '\0' the write system call should retrun success.

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:	Fri, 11 Sep 2015 17:05:31 +0800
From:	Sean Fu <fxinrong@...il.com>
To:	Steven Rostedt <rostedt@...dmis.org>
Cc:	"Eric W. Biederman" <ebiederm@...ssion.com>,
	Andrew Morton <akpm@...ux-foundation.org>,
	Austin S Hemmelgarn <ahferroin7@...il.com>,
	Andrey Ryabinin <ryabinin.a.a@...il.com>,
	Ulrich Obergfell <uobergfe@...hat.com>,
	Prarit Bhargava <prarit@...hat.com>,
	Eric B Munson <emunson@...mai.com>,
	"Paul E. McKenney" <paulmck@...ux.vnet.ibm.com>,
	Johannes Weiner <hannes@...xchg.org>,
	Thomas Gleixner <tglx@...utronix.de>,
	Don Zickus <dzickus@...hat.com>,
	Heinrich Schuchardt <xypron.glpk@....de>,
	David Rientjes <rientjes@...gle.com>,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH] kernel/sysctl.c: If "count" including the terminating
 byte '\0' the write system call should retrun success.

On Wed, Sep 9, 2015 at 12:36 AM, Steven Rostedt <rostedt@...dmis.org> wrote:
> On Tue, 08 Sep 2015 11:19:14 -0500
> ebiederm@...ssion.com (Eric W. Biederman) wrote:
>
>
>> This patch does not implement the old behavior.
>>
>> The old code does use '\0' as a buffer terminator, and because it does
>> not check things closely I can see how it could accept a '\0' from
>> userspace and treat that as an early buffer terminator.
>>
>> The patch treats '\0' as a number separator and allows things that have
>> never been allowed before and quite frankly is very scary as it just
>> invites bugs.
>>
>> So I do not think we should merge the given patch.  It is just wrong.
>> One that simply truncates the input buffer at the first '\0' character I
>> think we can consider, although I am not a fan.
>
> I agree, and was thinking this patch did that, but I didn't look close
> enough (why I never gave a Reviewed-by to it either).
>
>
>>
>> Steve as far as what I think would break.  I don't think the current
>> behavior should have broken anything and apparently it did.  I don't see
>> what a change that simply truncates the buffer at the first embedded
>> '\0' would break, but I don't know how to test that there isn't anything
>> that it will.  We are way past the point of reasonable expectations
>> being able to guide us.  4 years should have been more than enough soak
>> time to have been able to say that the change was good, but apparently
>> it was not.
>
> Well, to be fair, a lot of people (distros, etc) do not use the most
> recent kernels. 4 years may be the first time a tool touches a kernel.
>
>>
>> My gut feel says that if we are going to change this, at this late date,
>> we find the one specific proc file that matters and change it just for
>> that one proc file, and in that change we treat '\0' as a terminator not
>> as a separator.  I never did see in the conversation which proc file it
>> is that actually matters.  The principle is that the more precise and
>> the more localized such a change is the less chance it has of causing a
>> regression of something else, and the greater the chance we can look at
>> a specific issue.
>
> Sounds like a reasonable compromise. Sean, can you make a patch that
> only affects the one proc file (comment it well in the code), and have
> it accept nothing past the '\0'. Even if someone passed in "1 \0 2", it
> would only see "1 "
The current code uses uniform handler (e.g. "proc_dointvec") for all
same type proc file.
So all integer type proc file are affected.
In fact, The behavior of all integer type proc file should be changed.
>
> -- Steve
--
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/