From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754576AbbIMQwH (ORCPT ); Sun, 13 Sep 2015 12:52:07 -0400 Received: from out03.mta.xmission.com ([166.70.13.233]:52483 "EHLO out03.mta.xmission.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753236AbbIMQwF (ORCPT ); Sun, 13 Sep 2015 12:52:05 -0400 From: ebiederm@xmission.com (Eric W. Biederman) To: Sean Fu Cc: Steven Rostedt , 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: <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> <20150908123616.09589bfe@gandalf.local.home> <87mvwsubz1.fsf@x220.int.ebiederm.org> Date: Sun, 13 Sep 2015 11:44:10 -0500 In-Reply-To: (Sean Fu's message of "Sun, 13 Sep 2015 20:39:31 +0800") Message-ID: <87mvwql15x.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: U2FsdGVkX19E5U705009bHd6LV+f4xngPzkNfbIFSOU= 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.0 TVD_RCVD_IP Message was received from an IP address * 0.7 XMSubLong Long Subject * 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 * [sa05 1397; Body=1 Fuz1=1 Fuz2=1] * 0.0 T_TooManySym_03 6+ unique symbols in subject * 0.0 T_TooManySym_01 4+ unique symbols in subject * 0.0 T_TooManySym_02 5+ unique symbols in subject X-Spam-DCC: XMission; sa05 1397; Body=1 Fuz1=1 Fuz2=1 X-Spam-Combo: ;Sean Fu X-Spam-Relay-Country: X-Spam-Timing: total 950 ms - load_scoreonly_sql: 0.05 (0.0%), signal_user_changed: 4.9 (0.5%), b_tie_ro: 3.6 (0.4%), parse: 3.7 (0.4%), extract_message_metadata: 14 (1.5%), get_uri_detail_list: 2.4 (0.3%), tests_pri_-1000: 6 (0.7%), tests_pri_-950: 1.32 (0.1%), tests_pri_-900: 1.03 (0.1%), tests_pri_-400: 34 (3.6%), check_bayes: 32 (3.4%), b_tokenize: 9 (0.9%), b_tok_get_all: 11 (1.1%), b_comp_prob: 4.6 (0.5%), b_tok_touch_all: 5.0 (0.5%), b_finish: 1.11 (0.1%), tests_pri_0: 873 (91.9%), tests_pri_500: 6 (0.6%), 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 in02.mta.xmission.com) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Sean Fu writes: > On Sat, Sep 12, 2015 at 1:01 AM, Eric W. Biederman > wrote: >> Sean Fu writes: >> >>>> 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. >> >> No. I do not believe the proprietary binary application you are dealing >> with writes to all proc files that use the proc_dointvec handler. > I means all ctl_table whose .proc_handler is "proc_dointvec" are > affected. I mean this only deserves consideration because this is a regression report. I mean by limiting this to only the proc files that are written to by the weird program that broke we can minimize the chances that anything else will break. I do not believe that the weird program that broke writes to every proc file. Certainly I have not heard that asserted. >>> In fact, The behavior of all integer type proc file should be changed. >> >> Not at all. The only files that we can possibly justify changing today >> are the files where an actual regression is being observed. >> >> Because quite frankly 5 years is way too long to wait to report a >> regression. By and large software is reasonable and treats proc >> files as text files where '\0' is an invalid character. > 5 years is not enough long for distros, specially enterprise distros. > The most of HuaWei machines run our SLES10sp3(2.6.16, SUSE LINUX > ENTERPRISE SERVER). > They use one enterprise version for 5+ years usually. If you want to play by enterprise kernel rules please talk to your enterprise kernel support people. >> Accepting a '\0' is not at all reasonable for a text interface. The >> application that does it is buggy. > It is hard to comprehend that the current kernel can accept two bytes > "1 ", "1\t", "1\n" except "1\0". '\0' is not and has never been valid in a text file. proc files are a text interface. Expecting '\0' to be accepted is very strange, and apparently there is only one program in existence that does. That a trailing '\0' was ever accepted was due to a bug in the code. Accepting '\0' in general in a text interface is a very dangerous and buggy pattern so it must be done very carefully or else other regressions or bugs could be easily introduced. That no one has complained about this in the 5 years since the change happened strongly indicates this no one else cares. A very targeted very narrow regression fix that only handles a trailing '\0' and that only changes the behavior of proc files that matter is reasonable. Or do you volunteer to go out and test every program that has been written or updated to write to proc in the last 5 years (since the behavior changed) and verify that none of them in no circumstances depend upon failing if an trailing '\0' is included? If you can audit all of the code written in the last 5 years and verify that the change will not introduce problems for any other user space program we can talk about changing all of the proc files that use proc_dointvec. Eric