On Mon 12-02-24 14:31:10, Eric Sandeen wrote:
> On 2/9/24 1:43 PM, Eric Sandeen wrote:
> > Convert the UDF filesystem to the new mount API.
> >
> > UDF is slightly unique in that it always preserves prior mount
> > options across a remount, so that's handled by udf_init_options().
> >
> > Signed-off-by: Eric Sandeen <sandeen@xxxxxxxxxx>
...
> > + case Opt_gid:
> > + if (0 == kstrtoint(param->string, 10, &uv)) {
> > uopt->gid = make_kgid(current_user_ns(), uv);
> > if (!gid_valid(uopt->gid))
> > - return 0;
> > + return -EINVAL;
> > uopt->flags |= (1 << );
> > - break;
> > - case Opt_uid:
> > - if (match_uint(args, &uv))
> > - return 0;
> > + } else if (!strcmp(param->string, "forget")) {
> > + uopt->flags |= (1 << UDF_FLAG_GID_FORGET);
> > + } else if (!strcmp(param->string, "ignore")) {
> > + /* this option is superseded by gid=<number> */
> > + ;
> > + } else {
> > + return -EINVAL;
> > + }
> > + break;
>
> I wonder if I need to redo this and not directly set the make_kgid option
> into uopt->gid. We do test that uopt->gid is valid, and return an error, and
> skip setting UDF_FLAG_GID_SET, but ...
>
> ...
>
> > -static int udf_fill_super(struct super_block *sb, void *options, int silent)
> > +static int udf_fill_super(struct super_block *sb, struct fs_context *fc)
> > {
> > int ret = -EINVAL;
> > struct inode *inode = NULL;
> > - struct udf_options uopt;
> > + struct udf_options *uopt = fc->fs_private;
> > struct kernel_lb_addr rootdir, fileset;
> > struct udf_sb_info *sbi;
> > bool lvid_open = false;
> > -
> > - uopt.flags = (1 << UDF_FLAG_USE_AD_IN_ICB) | (1 << UDF_FLAG_STRICT);
> > - /* By default we'll use overflow[ug]id when UDF inode [ug]id == -1 */
> > - uopt.uid = make_kuid(current_user_ns(), overflowuid);
> > - uopt.gid = make_kgid(current_user_ns(), overflowgid);
>
> this initialization (now moved to udf_init_options) gets overwritten
> even if the [gu]id was invalid during parsing ...
>
> > + sbi->s_flags = uopt->flags;
> > + sbi->s_uid = uopt->uid;
> > + sbi->s_gid = uopt->gid;
>
> ... and gets set into sbi here.
>
> In the past (I think) the whole mount would fail with an invalid UID/GID but w/
> fsconfig, we could just fail that one config and continue with the rest.
I see. Yes, I guess it cannot happen with normal mount but let's be
defensive and make sure only valid uid/gid gets into uopt and sbi.
Honza
--
Jan Kara <jack@xxxxxxxx>
SUSE Labs, CR
