From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752899AbcDSWRY (ORCPT ); Tue, 19 Apr 2016 18:17:24 -0400 Received: from out01.mta.xmission.com ([166.70.13.231]:59496 "EHLO out01.mta.xmission.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752447AbcDSWRW (ORCPT ); Tue, 19 Apr 2016 18:17:22 -0400 From: ebiederm@xmission.com (Eric W. Biederman) To: Linus Torvalds Cc: "H. Peter Anvin" , Andy Lutomirski , security@debian.org, "security\@kernel.org" , Al Viro , "security\@ubuntu.com \>\> security" , Peter Hurley , Serge Hallyn , Willy Tarreau , Aurelien Jarno , One Thousand Gnomes , Jann Horn , Greg KH , Linux Kernel Mailing List , Jiri Slaby , Florian Weimer References: <878u0s3orx.fsf_-_@x220.int.ebiederm.org> <87twjcorwg.fsf@x220.int.ebiederm.org> <20160409140909.42315e6d@lxorguk.ukuu.org.uk> <83FE8CD2-C0A2-4ADB-AEBD-8DD89AD4F88A@zytor.com> <87bn5ij0x1.fsf@x220.int.ebiederm.org> <78205895-E11D-417F-91DC-4BCA0B61A122@zytor.com> <570D4781.3070600@zytor.com> <877ffyzy1j.fsf_-_@x220.int.ebiederm.org> Date: Tue, 19 Apr 2016 17:06:33 -0500 In-Reply-To: (Linus Torvalds's message of "Sat, 16 Apr 2016 11:31:14 -0700 (PDT)") Message-ID: <87twixgsnq.fsf@x220.int.ebiederm.org> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/24.5 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain X-XM-AID: U2FsdGVkX19zRiGcHvRLuVvCXXAIoKu/bI2V8XNm0pw= X-SA-Exim-Connect-IP: 97.119.105.151 X-SA-Exim-Mail-From: ebiederm@xmission.com X-Spam-Report: * -1.0 ALL_TRUSTED Passed through trusted hosts only via SMTP * 1.5 XMNoVowels Alpha-numberic number with no vowels * 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 * [sa05 1397; Body=1 Fuz1=1 Fuz2=1] X-Spam-DCC: XMission; sa05 1397; Body=1 Fuz1=1 Fuz2=1 X-Spam-Combo: *;Linus Torvalds X-Spam-Relay-Country: X-Spam-Timing: total 320 ms - load_scoreonly_sql: 0.03 (0.0%), signal_user_changed: 4.3 (1.3%), b_tie_ro: 3.0 (1.0%), parse: 1.41 (0.4%), extract_message_metadata: 15 (4.5%), get_uri_detail_list: 2.5 (0.8%), tests_pri_-1000: 4.6 (1.4%), tests_pri_-950: 0.90 (0.3%), tests_pri_-900: 0.80 (0.3%), tests_pri_-400: 28 (8.6%), check_bayes: 26 (8.3%), b_tokenize: 8 (2.6%), b_tok_get_all: 10 (3.1%), b_comp_prob: 2.6 (0.8%), b_tok_touch_all: 3.6 (1.1%), b_finish: 0.64 (0.2%), tests_pri_0: 257 (80.2%), check_dkim_signature: 0.42 (0.1%), check_dkim_adsp: 4.8 (1.5%), tests_pri_500: 6 (1.9%), rewrite_mail: 0.00 (0.0%) Subject: Re: [PATCH 01/16] devpts: Attempting to get it right 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 Linus Torvalds writes: > So I've looked at the devpts patches some more, and I'm still not happy > with them. > > And one thing I really detest about them is that the 16-patch series > didn't really make me warm and fuzzy in general. Some of the patches were > fine, but on the whole it all still looked rather hacky. > > So I started looking at the code with the intent of trying to clean things > up _gradually_, knowing roughly where we want to end up, but also trying > to make single patches that look sane on their own, and can be judged on > their own without any other patches or even any semantic arguments. > > And this appended patch is I think the first step. > > What this does is get rid of the horrible notion of having that > > struct inode *ptmx_inode > > be the interface between the pty code and devpts. By de-emphasizing the > ptmx inode, a lot of things actually get cleaner, and we will have a much > saner way forward. > > The patch itself is actually fairly straightforward, and apart from some > locking cleanups it's pretty mechanical: > > - the interfaces that devpts exposes all take "struct pts_fs_info *" > instead of "struct inode *ptmx_inode" now. > > NOTE! The "struct pts_fs_info" thing is a completely opaque structure > as far as the pty driver is concerned: it's still declared entirely > internally to devpts. So the pty code can't actually access it in any > way, just pass it as a "cookie" to the devpts code. > > - the "look up the pts fs info" is now a single clear operation, that > also does the reference count increment on the pts superblock. > > So "devpts_add/del_ref()" is gone, and replaced by a "lookup and get > ref" operation (devpts_get_ref(inode)), along with a "put ref" op > (devpts_put_ref()). > > - the pty master "tty->driver_data" field now contains the pts_fs_info, > not the ptmx inode. > > - because we don't care about the ptmx inode any more as some kind of > base index, the ref counting can now drop the inode games - it just > gets the ref on the superblock. > > - the pts_fs_info now has a back-pointer to the super_block. That's so > that we can easily look up the information we actually need. Although > quite often, the pts fs info was actually all we wanted, and not having > to look it up based on some magical inode makes things more > straightforward. > > Now, I haven't actually *tested* the patch very much: it compiles, it > boots, and my (fairly limited) environment still works, but that's by no > means exhaustive. So I may have screwed something up, but most of this was > really fairly straightforward. > > But more importantly, I think it all makes sense independently of anything > else. In particular, now that "devpts_get_ref(inode)" operation should > really be the *only* place we need to look up what devpts instance we're > associated with, and we do it exactly once, at ptmx_open() time. > > So I think this is a good step forward, while avoiding anything that could > be at all controversial. > > Comments about the patch? Mostly I think it is six of one half dozen of the other, but given driver_data is used so few times and that those times are very clear whether we are accessing a master or a slave worth doing just for clarity. I have work inspired by this rolled into my code. I will post shortly after a little more testing. Eric