From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755370AbbIHQgW (ORCPT ); Tue, 8 Sep 2015 12:36:22 -0400 Received: from smtprelay0181.hostedemail.com ([216.40.44.181]:39304 "EHLO smtprelay.hostedemail.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1754781AbbIHQgU (ORCPT ); Tue, 8 Sep 2015 12:36:20 -0400 X-Session-Marker: 726F737465647440676F6F646D69732E6F7267 X-Spam-Summary: 2,0,0,,d41d8cd98f00b204,rostedt@goodmis.org,:::::::::::::::::::::::::::::,RULES_HIT:41:355:379:541:599:800:973:988:989:1260:1277:1311:1313:1314:1345:1359:1437:1515:1516:1518:1534:1542:1593:1594:1711:1730:1747:1777:1792:2393:2553:2559:2562:2693:3138:3139:3140:3141:3142:3355:3622:3865:3866:3867:3868:3870:3871:3872:3873:3874:5007:6119:6261:6742:7875:10004:10400:10471:10848:10967:11232:11658:11914:12517:12519:12663:12740:13255:13891:21080:21212,0,RBL:none,CacheIP:none,Bayesian:0.5,0.5,0.5,Netcheck:none,DomainCache:0,MSF:not bulk,SPF:fn,MSBL:0,DNSBL:none,Custom_rules:0:0:0 X-HE-Tag: note49_19e17c974e113 X-Filterd-Recvd-Size: 4659 Date: Tue, 8 Sep 2015 12:36:16 -0400 From: Steven Rostedt To: ebiederm@xmission.com (Eric W. Biederman) Cc: Sean Fu , Andrew Morton , Austin S Hemmelgarn , Andrey Ryabinin , Ulrich Obergfell , Prarit Bhargava , Eric B Munson , "Paul E. McKenney" , Johannes Weiner , Thomas Gleixner , Don Zickus , Heinrich Schuchardt , David Rientjes , linux-kernel@vger.kernel.org Subject: Re: [PATCH] kernel/sysctl.c: If "count" including the terminating byte '\0' the write system call should retrun success. Message-ID: <20150908123616.09589bfe@gandalf.local.home> In-Reply-To: <87si6oq3y5.fsf@x220.int.ebiederm.org> References: <9CF68C3D-499C-4AFC-B890-BA5ECA284347@xmission.com> <20150825101507.4e3be4ca@gandalf.local.home> <55DCA705.8000903@gmail.com> <20150825150536.546a73e5@gandalf.local.home> <20150826163621.28d7990a@gandalf.local.home> <20150826223230.4348d936@grimm.local.home> <20150908111730.6a1ecca6@gandalf.local.home> <87si6oq3y5.fsf@x220.int.ebiederm.org> X-Mailer: Claws Mail 3.11.1 (GTK+ 2.24.28; 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 Tue, 08 Sep 2015 11:19:14 -0500 ebiederm@xmission.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 " -- Steve