From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by smtp.lore.kernel.org (Postfix) with ESMTP id 7A1D2C3064D for ; Fri, 28 Jun 2024 13:45:05 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id B87B36B0089; Fri, 28 Jun 2024 09:45:04 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id B35FA6B008C; Fri, 28 Jun 2024 09:45:04 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id A24946B0092; Fri, 28 Jun 2024 09:45:04 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0015.hostedemail.com [216.40.44.15]) by kanga.kvack.org (Postfix) with ESMTP id 84B836B0089 for ; Fri, 28 Jun 2024 09:45:04 -0400 (EDT) Received: from smtpin11.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay07.hostedemail.com (Postfix) with ESMTP id 3796D160394 for ; Fri, 28 Jun 2024 13:45:02 +0000 (UTC) X-FDA: 82280418444.11.989A8AA Received: from sandeen.net (sandeen.net [63.231.237.45]) by imf07.hostedemail.com (Postfix) with ESMTP id 273CD40007 for ; Fri, 28 Jun 2024 13:44:58 +0000 (UTC) Authentication-Results: imf07.hostedemail.com; dkim=pass header.d=sandeen.net header.s=default header.b=qrx9Vpyh; spf=pass (imf07.hostedemail.com: domain of sandeen@sandeen.net designates 63.231.237.45 as permitted sender) smtp.mailfrom=sandeen@sandeen.net; dmarc=pass (policy=reject) header.from=sandeen.net ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1719582280; h=from:from:sender:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:cc:mime-version:mime-version: content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references:dkim-signature; bh=Wm0ob2liKwaqVbn00ikDKfK69kD3HD4g+HnBrpsBtW0=; b=jxc4wH1fwFg/F5mhWE86hCXvNOr9oX8Keb2M/BVdT7YQIxd2UmskesbzOHq3fFav0P9JfL v1+dFJsxZOy4/5DpUs3VYWOwVF1KJReap9pqsYwmWtAroEsaJWYFMN737wwZtCLOHoR/qN hkzA4UqQECTMBVzUISxqDUzjqIrPWrQ= ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1719582280; a=rsa-sha256; cv=none; b=tLe0qdhA5KCs7J8q1oD+fh0VyArXv64qxz/sYAUtdjB0lzHLXeW43j81el/tI+7h6wEwH3 voT58IYBfx/OZzskOOWgGb9jGVMzQG4aeWadZo3/go7oT0lKP94VBN/TN9ts1VJoANHZzf aeq4nMbHVHg9eOGrtux1t2X4dFP54I0= ARC-Authentication-Results: i=1; imf07.hostedemail.com; dkim=pass header.d=sandeen.net header.s=default header.b=qrx9Vpyh; spf=pass (imf07.hostedemail.com: domain of sandeen@sandeen.net designates 63.231.237.45 as permitted sender) smtp.mailfrom=sandeen@sandeen.net; dmarc=pass (policy=reject) header.from=sandeen.net Received: from [10.0.0.71] (usg [10.0.0.1]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by sandeen.net (Postfix) with ESMTPSA id B8504479AE7; Fri, 28 Jun 2024 08:44:57 -0500 (CDT) DKIM-Filter: OpenDKIM Filter v2.11.0 sandeen.net B8504479AE7 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=sandeen.net; s=default; t=1719582297; bh=Wm0ob2liKwaqVbn00ikDKfK69kD3HD4g+HnBrpsBtW0=; h=Date:Subject:To:Cc:References:From:In-Reply-To:From; b=qrx9Vpyhxcf6Ccxu1nuAmtF93uW/ueFYWzOccKUh54TalNBQ4t63BP5oUyWlboQhC WX+PUV2Brzf30rbzUiP594AXcjW3QoOLhz3z+XQQQWuRssk79igSO2coEA6/JvL6bf BTNwcp6VJJqsWJEZ3EbAWEh/xCJlB7Lf/ONSJtCBE97BxFAWjQA9bdLHG1KDnltvdT 0+RFws1BZFMMBR0ickPmjzBuqw4WAT0BG53yvl+IW8OXJCu7NGrVSBWc8pGmx+Y18r 24NKBfo0ZsCxWTtD9jxdMbrx9h+VqakxBVi/gbrwKM/r6xSc6gKvi9JHqtHgszm9vJ DhHGFwa2Bx1/Q== Message-ID: <7c90924d-b023-4fa7-801d-ea0a53a5e5ed@sandeen.net> Date: Fri, 28 Jun 2024 08:44:57 -0500 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH 01/14] fs_parse: add uid & gid option option parsing helpers To: Jan Kara , Eric Sandeen Cc: linux-fsdevel@vger.kernel.org, Christian Brauner , autofs@vger.kernel.org, "Rafael J. Wysocki" , linux-efi@vger.kernel.org, Namjae Jeon , linux-ext4@vger.kernel.org, Miklos Szeredi , linux-mm@kvack.org, ntfs3@lists.linux.dev, linux-cifs@vger.kernel.org, linux-trace-kernel@vger.kernel.org, Hans Caniullan , Alexander Viro References: <8dca3c11-99f4-446d-a291-35c50ed2dc14@redhat.com> <20240628094517.ifs4bp73nlggsnxz@quack3> Content-Language: en-US From: Eric Sandeen In-Reply-To: <20240628094517.ifs4bp73nlggsnxz@quack3> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-Stat-Signature: qm9r34yg33tqqmufoap6nyqoa4tta3kn X-Rspamd-Queue-Id: 273CD40007 X-Rspam-User: X-Rspamd-Server: rspam08 X-HE-Tag: 1719582298-511353 X-HE-Meta: U2FsdGVkX1/Vl/pyh57RsuvKH+TzqCJJONU/sVRD/9NkJRuk/pNvU8IjOxAQKGFrBY1Iy7VuHUIiJ+7DcnPU5NtkmWOSzl0JL0DUjRojXg9VXOp2fqrkUpOxjiOaLICd+BpMsx8Way+/VZfnjqrNDKDSbLk5PYjYRItz62Lq1+TMlwxcDRHIaafy201RXzenv0WK8NkWJulZyAxcA+GjqupayJpcZ5qXaTinKUUpbpe/4daC9BBJhU2G/Prgp4cAyzniOR/xfU9XCPzdUOJ9KaqhN7X9y0RagXIIG1wUb86Z0K0F63lKU5uvP85BCtWVOmOhnbWYjRwpQbuDuTARk0zRBB4xGj23IjdhW2tIGvnJiC+c4kjGMGj0RZdUlDMPHi3luuCzoFmZAsN5D0wO1m1ASqptaMRtwjgU3XYXtgPvA1DRG0vGxrj15Tf7h2Q2191GxPlQDPedXUZtPbq2gYamKolNWfcgou3zja54jgDlHHLQ8T3sTA/fHTIbUTEkre5ovzJkwERzweyqCkyM4oYJ6W0xDvcRI/Q0j6z9Cs6Ow4FOevv+ePuV5ajDE11HzBevgGCahQxSnKh+SNLVfxAIWV2AoluvkMHLq23+GM4wswHRhgXGwQ1pL49nwg9wbcB6QtHi3DMgL3l4JTKkSEJtYMeYbJ6VqSLtDVL0GX9D8OVpxXIbnPwkc8WeThyFYhmOrwngYPOCXuYhOEu3RVBoUwUNXQmXlRzx9EwNKhm/roCpdZwpgRyqs26SRf78m4FPHp5CD4dY6UVEq+AeKCnbct/K7cBeB816850xet6DQP/Dy0TG8gJpi7mjUzhwwKSvFzq0p/jGxb5bECkGDydmBc0MWxeWd2si5zCCWyzJUspcaE7/kpZmzWfV/h936aHwuzL0wmRcGC6qBuwL/uuu2ZRRgAIOuMLx8+8ZmFMirqSXDhlG9b2zGU85AunflEcdb/ZhRPIe9xs7tIV F7Le63yu svBBUWfjiUIK7tu4IRommVo1uqfEWODRNVSsyvfednb2tLE76RiqaICU9F8ThkguUF7hSzEi+ilLZNgZAoDiqti/7YudZ3ub7wO2/gofblj6yxuPkJaSlwJqR/bYJZzHhMzvoJpWdhwmcpZ3Q0qKGIFpJK+w9bQbQRQ8B8hRbtNSR2f7VLt5KsmaZbqImsH9H59kY1sL+grdpjIjh55Fxcwzhwfy8knVoxT2mCKPCUltPGdlmFp5tF0dLD1To/j6ATTAM8Sc0OtlA4N2qmF72sdBPBaz3qXLb51cT42EHU5SS0SNKXRUZ9Jv3S/kNPRKTzLDQZdnoTQNDf0shd86K8Byg5UxpPrYZ/FJlquIZHUc9YZZELsnuis1noZzN11LFlwovdIDPmiTlzcJ+LuNDvctFN1fg6MHri9NBnT3TnOr+0WOrm+Gi2s46Mpd3EQhQOTIT X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: List-Subscribe: List-Unsubscribe: On 6/28/24 4:45 AM, Jan Kara wrote: > On Thu 27-06-24 19:26:24, Eric Sandeen wrote: >> Multiple filesystems take uid and gid as options, and the code to >> create the ID from an integer and validate it is standard boilerplate >> that can be moved into common helper functions, so do that for >> consistency and less cut&paste. >> >> This also helps avoid the buggy pattern noted by Seth Jenkins at >> https://lore.kernel.org/lkml/CALxfFW4BXhEwxR0Q5LSkg-8Vb4r2MONKCcUCVioehXQKr35eHg@mail.gmail.com/ >> because uid/gid parsing will fail before any assignment in most >> filesystems. >> >> Signed-off-by: Eric Sandeen > > I like the idea since this seems like a nobrainer but is actually > surprisingly subtle... > >> diff --git a/fs/fs_parser.c b/fs/fs_parser.c >> index a4d6ca0b8971..24727ec34e5a 100644 >> --- a/fs/fs_parser.c >> +++ b/fs/fs_parser.c >> @@ -308,6 +308,40 @@ int fs_param_is_fd(struct p_log *log, const struct fs_parameter_spec *p, >> } >> EXPORT_SYMBOL(fs_param_is_fd); >> >> +int fs_param_is_uid(struct p_log *log, const struct fs_parameter_spec *p, >> + struct fs_parameter *param, struct fs_parse_result *result) >> +{ >> + kuid_t uid; >> + >> + if (fs_param_is_u32(log, p, param, result) != 0) >> + return fs_param_bad_value(log, param); >> + >> + uid = make_kuid(current_user_ns(), result->uint_32); > > But here is the problem: Filesystems mountable in user namespaces need to use > fc->user_ns for resolving uids / gids (e.g. like fuse_parse_param()). > Having helpers that work for some filesystems and are subtly broken for > others is worse than no helpers... Or am I missing something? Yeah, I should have pointed that out. tmpfs still does that check after the initial trivial parsing after this change to use the basic helper: case Opt_uid: kuid = result.uid; /* * The requested uid must be representable in the * filesystem's idmapping. */ if (!kuid_has_mapping(fc->user_ns, kuid)) goto bad_value; ctx->uid = kuid; break; I can see your point about risks of a helper that doesn't cover all cases though. > And the problem with fc->user_ns is that currently __fs_parse() does not > get fs_context as an argument... So that will need some larger work. Yup, this was discussed a little when I sent this idea as an RFC, and the (brief/small) consensus was that it was worth going this far for now. Getting fc back into __fs_parse looks rather tricky and Al was not keen on the idea, for some reason. Thanks, -Eric > Honza