On Thu, 2025-01-30 at 13:15 +0100, Miklos Szeredi wrote: > Just like it's normal for unset values to be zero, unset strings should be > empty instead of containing random values. > > It seems to be a typical mistake that the mask returned by statmount is not > checked, which can result in various bugs. > > With this fix, these bugs are prevented, since it is highly likely that > userspace would just want to turn the missing mask case into an empty > string anyway (most of the recently found cases are of this type). > > Link: https://lore.kernel.org/all/CAJfpegsVCPfCn2DpM8iiYSS5DpMsLB8QBUCHecoj6s0Vxf4jzg@xxxxxxxxxxxxxx/ > Fixes: 68385d77c05b ("statmount: simplify string option retrieval") > Fixes: 46eae99ef733 ("add statmount(2) syscall") > Cc: <stable@xxxxxxxxxxxxxxx> # v6.8 > Signed-off-by: Miklos Szeredi <mszeredi@xxxxxxxxxx> > --- > fs/namespace.c | 25 ++++++++++++++++--------- > 1 file changed, 16 insertions(+), 9 deletions(-) > > diff --git a/fs/namespace.c b/fs/namespace.c > index a3ed3f2980cb..9c4d307a82cd 100644 > --- a/fs/namespace.c > +++ b/fs/namespace.c > @@ -5191,39 +5191,45 @@ static int statmount_string(struct kstatmount *s, u64 flag) > size_t kbufsize; > struct seq_file *seq = &s->seq; > struct statmount *sm = &s->sm; > - u32 start = seq->count; > + u32 start, *offp; > + > + /* Reserve an empty string at the beginning for any unset offsets */ > + if (!seq->count) > + seq_putc(seq, 0); > + > + start = seq->count; > > switch (flag) { > case STATMOUNT_FS_TYPE: > - sm->fs_type = start; > + offp = &sm->fs_type; > ret = statmount_fs_type(s, seq); > break; > case STATMOUNT_MNT_ROOT: > - sm->mnt_root = start; > + offp = &sm->mnt_root; > ret = statmount_mnt_root(s, seq); > break; > case STATMOUNT_MNT_POINT: > - sm->mnt_point = start; > + offp = &sm->mnt_point; > ret = statmount_mnt_point(s, seq); > break; > case STATMOUNT_MNT_OPTS: > - sm->mnt_opts = start; > + offp = &sm->mnt_opts; > ret = statmount_mnt_opts(s, seq); > break; > case STATMOUNT_OPT_ARRAY: > - sm->opt_array = start; > + offp = &sm->opt_array; > ret = statmount_opt_array(s, seq); > break; > case STATMOUNT_OPT_SEC_ARRAY: > - sm->opt_sec_array = start; > + offp = &sm->opt_sec_array; > ret = statmount_opt_sec_array(s, seq); > break; > case STATMOUNT_FS_SUBTYPE: > - sm->fs_subtype = start; > + offp = &sm->fs_subtype; > statmount_fs_subtype(s, seq); > break; > case STATMOUNT_SB_SOURCE: > - sm->sb_source = start; > + offp = &sm->sb_source; > ret = statmount_sb_source(s, seq); > break; > default: > @@ -5251,6 +5257,7 @@ static int statmount_string(struct kstatmount *s, u64 flag) > > seq->buf[seq->count++] = '\0'; > sm->mask |= flag; > + *offp = start; > return 0; > } > That seems like a reasonable thing to do. Reviewed-by: Jeff Layton <jlayton@xxxxxxxxxx>
