From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754070AbdJaXrz (ORCPT ); Tue, 31 Oct 2017 19:47:55 -0400 Received: from out02.mta.xmission.com ([166.70.13.232]:49869 "EHLO out02.mta.xmission.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751347AbdJaXry (ORCPT ); Tue, 31 Oct 2017 19:47:54 -0400 From: ebiederm@xmission.com (Eric W. Biederman) To: Christian Brauner Cc: linux-kernel@vger.kernel.org, serge@hallyn.com, tycho@tycho.ws, Linux Containers References: <20171024220441.10235-1-christian.brauner@ubuntu.com> <20171024220441.10235-2-christian.brauner@ubuntu.com> <871sliubhj.fsf_-_@xmission.com> Date: Tue, 31 Oct 2017 18:47:43 -0500 In-Reply-To: <871sliubhj.fsf_-_@xmission.com> (Eric W. Biederman's message of "Tue, 31 Oct 2017 18:46:32 -0500") Message-ID: <87po92swv4.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=1e9gG3-0000Ut-SK;;;mid=<87po92swv4.fsf_-_@xmission.com>;;;hst=in01.mta.xmission.com;;;ip=174.19.78.123;;;frm=ebiederm@xmission.com;;;spf=neutral X-XM-AID: U2FsdGVkX192H0+5j1p0QtyJXYZ8Cjvbls7F1XFRv+E= 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.7 XMSubLong Long Subject * 1.5 XMNoVowels Alpha-numberic number with no vowels * 0.0 TVD_RCVD_IP Message was received from an IP address * 0.8 BAYES_50 BODY: Bayes spam probability is 40 to 60% * [score: 0.5000] * -0.0 DCC_CHECK_NEGATIVE Not listed in DCC * [sa07 1397; Body=1 Fuz1=1 Fuz2=1] X-Spam-DCC: XMission; sa07 1397; Body=1 Fuz1=1 Fuz2=1 X-Spam-Combo: **;Christian Brauner X-Spam-Relay-Country: X-Spam-Timing: total 5685 ms - load_scoreonly_sql: 0.04 (0.0%), signal_user_changed: 2.6 (0.0%), b_tie_ro: 1.73 (0.0%), parse: 1.07 (0.0%), extract_message_metadata: 17 (0.3%), get_uri_detail_list: 4.0 (0.1%), tests_pri_-1000: 8 (0.1%), tests_pri_-950: 1.23 (0.0%), tests_pri_-900: 1.05 (0.0%), tests_pri_-400: 34 (0.6%), check_bayes: 32 (0.6%), b_tokenize: 14 (0.2%), b_tok_get_all: 10 (0.2%), b_comp_prob: 2.3 (0.0%), b_tok_touch_all: 4.3 (0.1%), b_finish: 0.81 (0.0%), tests_pri_0: 393 (6.9%), check_dkim_signature: 0.68 (0.0%), check_dkim_adsp: 3.1 (0.1%), tests_pri_500: 5224 (91.9%), poll_dns_idle: 5217 (91.8%), rewrite_mail: 0.00 (0.0%) Subject: [PATCH 2/5] userns: Simplify the user and group mapping functions 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 Consolidate reading the number of extents and computing the return value in the map_id_down, map_id_range_down and map_id_range. This removal of one read of extents makes one smp_rmb unnecessary and makes the code safe it is executed during the map write. Reading the number of extents twice and depending on the result being the same is not safe, as it could be 0 the first time and > 5 the second time, which would lead to misinterpreting the union fields. The consolidation of the return value just removes a duplicate caluculation which should make it easier to understand and maintain the code. Signed-off-by: "Eric W. Biederman" --- kernel/user_namespace.c | 132 +++++++++++++++++++++--------------------------- 1 file changed, 58 insertions(+), 74 deletions(-) diff --git a/kernel/user_namespace.c b/kernel/user_namespace.c index c9904ee084c4..563a2981d7c7 100644 --- a/kernel/user_namespace.c +++ b/kernel/user_namespace.c @@ -256,28 +256,17 @@ static int cmp_map_id(const void *k, const void *e) * map_id_range_down_max - Find idmap via binary search in ordered idmap array. * Can only be called if number of mappings exceeds UID_GID_MAP_MAX_BASE_EXTENTS. */ -static u32 map_id_range_down_max(struct uid_gid_map *map, u32 id, u32 count) +static struct uid_gid_extent * +map_id_range_down_max(unsigned extents, struct uid_gid_map *map, u32 id, u32 count) { - u32 extents; - struct uid_gid_extent *extent; struct idmap_key key; key.map_up = false; key.count = count; key.id = id; - extents = map->nr_extents; - smp_rmb(); - - extent = bsearch(&key, map->forward, extents, - sizeof(struct uid_gid_extent), cmp_map_id); - /* Map the id or note failure */ - if (extent) - id = (id - extent->first) + extent->lower_first; - else - id = (u32) -1; - - return id; + return bsearch(&key, map->forward, extents, + sizeof(struct uid_gid_extent), cmp_map_id); } /** @@ -285,41 +274,43 @@ static u32 map_id_range_down_max(struct uid_gid_map *map, u32 id, u32 count) * Can only be called if number of mappings is equal or less than * UID_GID_MAP_MAX_BASE_EXTENTS. */ -static u32 map_id_range_down_base(struct uid_gid_map *map, u32 id, u32 count) +static struct uid_gid_extent * +map_id_range_down_base(unsigned extents, struct uid_gid_map *map, u32 id, u32 count) { - unsigned idx, extents; + unsigned idx; u32 first, last, id2; id2 = id + count - 1; /* Find the matching extent */ - extents = map->nr_extents; - smp_rmb(); for (idx = 0; idx < extents; idx++) { first = map->extent[idx].first; last = first + map->extent[idx].count - 1; if (id >= first && id <= last && (id2 >= first && id2 <= last)) - break; + return &map->extent[idx]; } - /* Map the id or note failure */ - if (idx < extents) - id = (id - first) + map->extent[idx].lower_first; - else - id = (u32) -1; - - return id; + return NULL; } static u32 map_id_range_down(struct uid_gid_map *map, u32 id, u32 count) { - u32 extents = map->nr_extents; + struct uid_gid_extent *extent; + unsigned extents = map->nr_extents; smp_rmb(); if (extents <= UID_GID_MAP_MAX_BASE_EXTENTS) - return map_id_range_down_base(map, id, count); + extent = map_id_range_down_base(extents, map, id, count); + else + extent = map_id_range_down_max(extents, map, id, count); - return map_id_range_down_max(map, id, count); + /* Map the id or note failure */ + if (extent) + id = (id - extent->first) + extent->lower_first; + else + id = (u32) -1; + + return id; } /** @@ -327,38 +318,40 @@ static u32 map_id_range_down(struct uid_gid_map *map, u32 id, u32 count) * Can only be called if number of mappings is equal or less than * UID_GID_MAP_MAX_BASE_EXTENTS. */ -static u32 map_id_down_base(struct uid_gid_map *map, u32 id) +static struct uid_gid_extent * +map_id_down_base(unsigned extents, struct uid_gid_map *map, u32 id) { - unsigned idx, extents; + unsigned idx; u32 first, last; /* Find the matching extent */ - extents = map->nr_extents; - smp_rmb(); for (idx = 0; idx < extents; idx++) { first = map->extent[idx].first; last = first + map->extent[idx].count - 1; if (id >= first && id <= last) - break; + return &map->extent[idx]; } - /* Map the id or note failure */ - if (idx < extents) - id = (id - first) + map->extent[idx].lower_first; - else - id = (u32) -1; - - return id; + return NULL; } static u32 map_id_down(struct uid_gid_map *map, u32 id) { - u32 extents = map->nr_extents; + struct uid_gid_extent *extent; + unsigned extents = map->nr_extents; smp_rmb(); if (extents <= UID_GID_MAP_MAX_BASE_EXTENTS) - return map_id_down_base(map, id); + extent = map_id_down_base(extents, map, id); + else + extent = map_id_range_down_max(extents, map, id, 1); - return map_id_range_down_max(map, id, 1); + /* Map the id or note failure */ + if (extent) + id = (id - extent->first) + extent->lower_first; + else + id = (u32) -1; + + return id; } /** @@ -366,48 +359,50 @@ static u32 map_id_down(struct uid_gid_map *map, u32 id) * Can only be called if number of mappings is equal or less than * UID_GID_MAP_MAX_BASE_EXTENTS. */ -static u32 map_id_up_base(struct uid_gid_map *map, u32 id) +static struct uid_gid_extent * +map_id_up_base(unsigned extents, struct uid_gid_map *map, u32 id) { - unsigned idx, extents; + unsigned idx; u32 first, last; /* Find the matching extent */ - extents = map->nr_extents; - smp_rmb(); for (idx = 0; idx < extents; idx++) { first = map->extent[idx].lower_first; last = first + map->extent[idx].count - 1; if (id >= first && id <= last) - break; + return &map->extent[idx]; } - /* Map the id or note failure */ - if (idx < extents) - id = (id - first) + map->extent[idx].first; - else - id = (u32) -1; - - return id; + return NULL; } /** * map_id_up_max - Find idmap via binary search in ordered idmap array. * Can only be called if number of mappings exceeds UID_GID_MAP_MAX_BASE_EXTENTS. */ -static u32 map_id_up_max(struct uid_gid_map *map, u32 id) +static struct uid_gid_extent * +map_id_up_max(unsigned extents, struct uid_gid_map *map, u32 id) { - u32 extents; - struct uid_gid_extent *extent; struct idmap_key key; key.map_up = true; key.count = 1; key.id = id; - extents = map->nr_extents; + return bsearch(&key, map->reverse, extents, + sizeof(struct uid_gid_extent), cmp_map_id); +} + +static u32 map_id_up(struct uid_gid_map *map, u32 id) +{ + struct uid_gid_extent *extent; + unsigned extents = map->nr_extents; smp_rmb(); - extent = bsearch(&key, map->reverse, extents, - sizeof(struct uid_gid_extent), cmp_map_id); + if (extents <= UID_GID_MAP_MAX_BASE_EXTENTS) + extent = map_id_up_base(extents, map, id); + else + extent = map_id_up_max(extents, map, id); + /* Map the id or note failure */ if (extent) id = (id - extent->lower_first) + extent->first; @@ -417,17 +412,6 @@ static u32 map_id_up_max(struct uid_gid_map *map, u32 id) return id; } -static u32 map_id_up(struct uid_gid_map *map, u32 id) -{ - u32 extents = map->nr_extents; - smp_rmb(); - - if (extents <= UID_GID_MAP_MAX_BASE_EXTENTS) - return map_id_up_base(map, id); - - return map_id_up_max(map, id); -} - /** * make_kuid - Map a user-namespace uid pair into a kuid. * @ns: User namespace that the uid is in -- 2.14.1