On Tue, Jun 04, 2024 at 12:17:39PM -0500, 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 parsing 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@xxxxxxxxxxxxxx/ > because uid/gid parsing will fail before any assignment in most > filesystems. > > With this in place, filesystem parsing is simplified, as in > the patch at > https://git.kernel.org/pub/scm/linux/kernel/git/sandeen/linux.git/commit/?h=mount-api-uid-helper&id=480d0d3c6699abfbb174b1bf2ab2bbeeec4fe911 > > Note that FS_USERNS_MOUNT filesystems still need to do additional > checking with k[ug]id_has_mapping(), I think. > > Thoughts? Is this useful / worthwhile? If so I can send a proper > 2-patch series ccing the dozen or so filesystems the 2nd patch will > touch. :) > > Signed-off-by: Eric Sandeen <sandeen@xxxxxxxxxx> > --- Seems worthwhile to me. Ideally you'd funnel through the fc->user_ns smh so we can do the k[ug]id_has_mapping() checks right in these parsing helpers. > Documentation/filesystems/mount_api.rst | 9 +++++++-- > fs/fs_parser.c | 34 +++++++++++++++++++++++++++++++++ > include/linux/fs_parser.h | 6 +++++- > 3 files changed, 46 insertions(+), 3 deletions(-) > > diff --git a/Documentation/filesystems/mount_api.rst b/Documentation/filesystems/mount_api.rst > index 9aaf6ef75eb53b..317934c9e8fcac 100644 > --- a/Documentation/filesystems/mount_api.rst > +++ b/Documentation/filesystems/mount_api.rst > @@ -645,6 +645,8 @@ The members are as follows: > fs_param_is_blockdev Blockdev path * Needs lookup > fs_param_is_path Path * Needs lookup > fs_param_is_fd File descriptor result->int_32 > + fs_param_is_uid User ID (u32) result->uid > + fs_param_is_gid Group ID (u32) result->gid > ======================= ======================= ===================== > > Note that if the value is of fs_param_is_bool type, fs_parse() will try > @@ -678,6 +680,8 @@ The members are as follows: > fsparam_bdev() fs_param_is_blockdev > fsparam_path() fs_param_is_path > fsparam_fd() fs_param_is_fd > + fsparam_uid() fs_param_is_uid > + fsparam_gid() fs_param_is_gid > ======================= =============================================== > > all of which take two arguments, name string and option number - for > @@ -784,8 +788,9 @@ process the parameters it is given. > option number (which it returns). > > If successful, and if the parameter type indicates the result is a > - boolean, integer or enum type, the value is converted by this function and > - the result stored in result->{boolean,int_32,uint_32,uint_64}. > + boolean, integer, enum, uid, or gid type, the value is converted by this > + function and the result stored in > + result->{boolean,int_32,uint_32,uint_64,uid,gid}. > > If a match isn't initially made, the key is prefixed with "no" and no > value is present then an attempt will be made to look up the key with the > diff --git a/fs/fs_parser.c b/fs/fs_parser.c > index a4d6ca0b8971e6..9c4e4984aae8a4 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); > + if (!uid_valid(uid)) > + return inval_plog(log, "Bad uid '%s'", param->string); > + > + result->uid = uid; > + return 0; > +} > +EXPORT_SYMBOL(fs_param_is_uid); > + > +int fs_param_is_gid(struct p_log *log, const struct fs_parameter_spec *p, > + struct fs_parameter *param, struct fs_parse_result *result) > +{ > + kgid_t gid; > + > + if (fs_param_is_u32(log, p, param, result) != 0) > + return fs_param_bad_value(log, param); > + > + gid = make_kgid(current_user_ns(), result->uint_32); > + if (!gid_valid(gid)) > + return inval_plog(log, "Bad gid '%s'", param->string); > + > + result->gid = gid; > + return 0; > +} > +EXPORT_SYMBOL(fs_param_is_gid); > + > int fs_param_is_blockdev(struct p_log *log, const struct fs_parameter_spec *p, > struct fs_parameter *param, struct fs_parse_result *result) > { > diff --git a/include/linux/fs_parser.h b/include/linux/fs_parser.h > index d3350979115f0a..6cf713a7e6c6fc 100644 > --- a/include/linux/fs_parser.h > +++ b/include/linux/fs_parser.h > @@ -28,7 +28,7 @@ typedef int fs_param_type(struct p_log *, > */ > fs_param_type fs_param_is_bool, fs_param_is_u32, fs_param_is_s32, fs_param_is_u64, > fs_param_is_enum, fs_param_is_string, fs_param_is_blob, fs_param_is_blockdev, > - fs_param_is_path, fs_param_is_fd; > + fs_param_is_path, fs_param_is_fd, fs_param_is_uid, fs_param_is_gid; > > /* > * Specification of the type of value a parameter wants. > @@ -57,6 +57,8 @@ struct fs_parse_result { > int int_32; /* For spec_s32/spec_enum */ > unsigned int uint_32; /* For spec_u32{,_octal,_hex}/spec_enum */ > u64 uint_64; /* For spec_u64 */ > + kuid_t uid; > + kgid_t gid; > }; > }; > > @@ -131,6 +133,8 @@ static inline bool fs_validate_description(const char *name, > #define fsparam_bdev(NAME, OPT) __fsparam(fs_param_is_blockdev, NAME, OPT, 0, NULL) > #define fsparam_path(NAME, OPT) __fsparam(fs_param_is_path, NAME, OPT, 0, NULL) > #define fsparam_fd(NAME, OPT) __fsparam(fs_param_is_fd, NAME, OPT, 0, NULL) > +#define fsparam_uid(NAME, OPT) __fsparam(fs_param_is_uid, NAME, OPT, 0, NULL) > +#define fsparam_gid(NAME, OPT) __fsparam(fs_param_is_gid, NAME, OPT, 0, NULL) > > /* String parameter that allows empty argument */ > #define fsparam_string_empty(NAME, OPT) \ > -- > cgit 1.2.3-korg >
