From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754260AbdKALQG (ORCPT ); Wed, 1 Nov 2017 07:16:06 -0400 Received: from out03.mta.xmission.com ([166.70.13.233]:58246 "EHLO out03.mta.xmission.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751638AbdKALQF (ORCPT ); Wed, 1 Nov 2017 07:16:05 -0400 From: ebiederm@xmission.com (Eric W. Biederman) To: Christian Brauner Cc: Christian Brauner , Linux Kernel Mailing List , Serge Hallyn , Tycho Andersen , Linux Containers References: <20171024220441.10235-1-christian.brauner@ubuntu.com> <20171024220441.10235-2-christian.brauner@ubuntu.com> <871sliubhj.fsf_-_@xmission.com> Date: Wed, 01 Nov 2017 06:15:53 -0500 In-Reply-To: (Christian Brauner's message of "Wed, 1 Nov 2017 11:51:17 +0100") Message-ID: <87tvyemeqe.fsf@xmission.com> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/25.1 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain X-XM-SPF: eid=1e9r02-0003NS-Fa;;;mid=<87tvyemeqe.fsf@xmission.com>;;;hst=in01.mta.xmission.com;;;ip=174.19.78.123;;;frm=ebiederm@xmission.com;;;spf=neutral X-XM-AID: U2FsdGVkX19BSV0GYnKC7BFL4qd7GVwpEppcuYykVVA= X-SA-Exim-Connect-IP: 174.19.78.123 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 * 1.5 XMNoVowels Alpha-numberic number with no vowels * 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.4848] * -0.0 DCC_CHECK_NEGATIVE Not listed in DCC * [sa02 1397; Body=1 Fuz1=1 Fuz2=1] * 0.0 T_TooManySym_01 4+ unique symbols in subject X-Spam-DCC: XMission; sa02 1397; Body=1 Fuz1=1 Fuz2=1 X-Spam-Combo: *;Christian Brauner X-Spam-Relay-Country: X-Spam-Timing: total 5347 ms - load_scoreonly_sql: 0.09 (0.0%), signal_user_changed: 9 (0.2%), b_tie_ro: 7 (0.1%), parse: 2.4 (0.0%), extract_message_metadata: 33 (0.6%), get_uri_detail_list: 3.3 (0.1%), tests_pri_-1000: 13 (0.2%), tests_pri_-950: 2.4 (0.0%), tests_pri_-900: 1.90 (0.0%), tests_pri_-400: 36 (0.7%), check_bayes: 34 (0.6%), b_tokenize: 12 (0.2%), b_tok_get_all: 9 (0.2%), b_comp_prob: 6 (0.1%), b_tok_touch_all: 2.8 (0.1%), b_finish: 0.91 (0.0%), tests_pri_0: 477 (8.9%), check_dkim_signature: 1.91 (0.0%), check_dkim_adsp: 5 (0.1%), tests_pri_500: 4763 (89.1%), poll_dns_idle: 4744 (88.7%), rewrite_mail: 0.00 (0.0%) Subject: Re: [PATCH 0/5] userns: bump idmap limits, fixes & tweaks X-Spam-Flag: No X-SA-Exim-Version: 4.2.1 (built Thu, 05 May 2016 13:38:54 -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 Christian Brauner writes: > On Tue, Oct 31, 2017 at 06:46:32PM -0500, Eric W. Biederman wrote: >> >> Christian I have looked through your code and I have found one real >> issue and of things I want to twak > > Cool, thanks for taking a close look Eric. > >> >> The real issue is reading nr_extents multiple times when reading a map. >> That can introduce races that will allow walking past the end of the >> array, if the first read is 0 but the second read is > 5. >> >> I have also found a couple of tweaks that look like they are worth >> implementing. > > Yeah, I saw that you unified some of the functions. I was thinking about this > but wanted to keep the cases distinct even with some amount of code duplication. > But it seems very much worth it from a maintenance perspective. Thanks! Yes. If we have a performance regression I am willing to remove the unification of map_id_range_down and map_id_down. But I can't imagine that will result in a measurable performance difference. If it does make a measurable perforamnce difference we almost certainly need to split the bsearch case as well. >> As all of these are very small and very straight forward I have >> tested these and applied them all to my for-next branch > > Thanks for the fixes Eric. Really appreciated. If you're too swamped for stuff > like that I'm obviously happy to do such trivial fixes myself. :) If you would test this some more in your setup I would appreciate it, just in case I missed something. Given where we are in the development cycle and the correctness concerns I just applied these as without the fix for reading extents exactly once the code is dangerously wrong. Eric > Christian > >> >> >> Eric W. Biederman (5): >> userns: Don't special case a count of 0 >> userns: Simplify the user and group mapping functions >> userns: Don't read extents twice in m_start >> userns: Make map_id_down a wrapper for map_id_range_down >> userns: Simplify insert_extent >> >> kernel/user_namespace.c | 159 ++++++++++++++++-------------------------------- >> 1 file changed, 51 insertions(+), 108 deletions(-) >> >> >> >>