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
| ||
|
Date: Tue, 08 Sep 2015 11:19:14 -0500 From: ebiederm@...ssion.com (Eric W. Biederman) To: Steven Rostedt <rostedt@...dmis.org> Cc: Sean Fu <fxinrong@...il.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. Steven Rostedt <rostedt@...dmis.org> writes: > On Tue, 8 Sep 2015 11:11:38 +0800 > Sean Fu <fxinrong@...il.com> wrote: > >> On Fri, Aug 28, 2015 at 11:31 AM, Sean Fu <fxinrong@...il.com> wrote: >> > On Thu, Aug 27, 2015 at 4:32 PM, Sean Fu <fxinrong@...il.com> wrote: >> >> On Thu, Aug 27, 2015 at 10:32 AM, Steven Rostedt <rostedt@...dmis.org> wrote: >> >>> On Thu, 27 Aug 2015 08:17:29 +0800 >> >>> Sean Fu <fxinrong@...il.com> wrote: >> >>>> strace execute result: >> >>>> write(3, "1\2\0", 3) = -1 EINVAL (Invalid argument) >> > If vleft > 1, "1\0 2" is treated as invalid paraments and all string >> > include '\0' will be invalid. >> Hi All experts, >> Could you please signed off this patch? > > If anyone should take this, it would be Andrew. > > I have no issue with the patch. Eric, you had some issue, but I don't > see a scenario that would depend on the current behavior. That is, what > do you think would break if we put it back to the old behavior? 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. 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. 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. 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/
Powered by blists - more mailing lists