From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755441AbbIHQ0X (ORCPT ); Tue, 8 Sep 2015 12:26:23 -0400 Received: from out03.mta.xmission.com ([166.70.13.233]:55171 "EHLO out03.mta.xmission.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1755218AbbIHQ0W (ORCPT ); Tue, 8 Sep 2015 12:26:22 -0400 From: ebiederm@xmission.com (Eric W. Biederman) To: Steven Rostedt 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 References: <20150824125902.4ba11ec6@gandalf.local.home> <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> Date: Tue, 08 Sep 2015 11:19:14 -0500 In-Reply-To: <20150908111730.6a1ecca6@gandalf.local.home> (Steven Rostedt's message of "Tue, 8 Sep 2015 11:17:30 -0400") Message-ID: <87si6oq3y5.fsf@x220.int.ebiederm.org> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/24.3 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain X-XM-AID: U2FsdGVkX1+/VED/wMx0mDw9hpq0c6HhTMbnD7XUNhg= X-SA-Exim-Connect-IP: 67.3.201.231 X-SA-Exim-Mail-From: ebiederm@xmission.com X-Spam-Report: * -1.0 ALL_TRUSTED Passed through trusted hosts only via SMTP * 0.7 XMSubLong Long Subject * 0.0 TVD_RCVD_IP Message was received from an IP address * 0.0 T_TM2_M_HEADER_IN_MSG BODY: No description available. * 0.8 BAYES_50 BODY: Bayes spam probability is 40 to 60% * [score: 0.5000] * -0.0 DCC_CHECK_NEGATIVE Not listed in DCC * [sa06 1397; Body=1 Fuz1=1 Fuz2=1] * 0.2 T_XMDrugObfuBody_14 obfuscated drug references * 0.0 T_TooManySym_01 4+ unique symbols in subject * 0.0 T_TooManySym_03 6+ unique symbols in subject * 0.0 T_TooManySym_02 5+ unique symbols in subject X-Spam-DCC: XMission; sa06 1397; Body=1 Fuz1=1 Fuz2=1 X-Spam-Combo: ;Steven Rostedt X-Spam-Relay-Country: X-Spam-Timing: total 648 ms - load_scoreonly_sql: 0.06 (0.0%), signal_user_changed: 8 (1.2%), b_tie_ro: 5 (0.8%), parse: 2.1 (0.3%), extract_message_metadata: 19 (3.0%), get_uri_detail_list: 3.5 (0.5%), tests_pri_-1000: 8 (1.2%), tests_pri_-950: 1.59 (0.2%), tests_pri_-900: 1.26 (0.2%), tests_pri_-400: 29 (4.5%), check_bayes: 28 (4.3%), b_tokenize: 9 (1.4%), b_tok_get_all: 9 (1.4%), b_comp_prob: 3.2 (0.5%), b_tok_touch_all: 3.1 (0.5%), b_finish: 0.86 (0.1%), tests_pri_0: 566 (87.3%), tests_pri_500: 6 (1.0%), rewrite_mail: 0.00 (0.0%) Subject: Re: [PATCH] kernel/sysctl.c: If "count" including the terminating byte '\0' the write system call should retrun success. X-Spam-Flag: No X-SA-Exim-Version: 4.2.1 (built Wed, 24 Sep 2014 11:00:52 -0600) X-SA-Exim-Scanned: Yes (on in01.mta.xmission.com) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Steven Rostedt writes: > On Tue, 8 Sep 2015 11:11:38 +0800 > Sean Fu wrote: > >> On Fri, Aug 28, 2015 at 11:31 AM, Sean Fu wrote: >> > On Thu, Aug 27, 2015 at 4:32 PM, Sean Fu wrote: >> >> On Thu, Aug 27, 2015 at 10:32 AM, Steven Rostedt wrote: >> >>> On Thu, 27 Aug 2015 08:17:29 +0800 >> >>> Sean Fu 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