| Message ID | 20250130121500.113446-1-mszeredi@redhat.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | statmount: let unset strings be empty | expand |
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@mail.gmail.com/ > Fixes: 68385d77c05b ("statmount: simplify string option retrieval") > Fixes: 46eae99ef733 ("add statmount(2) syscall") > Cc: <stable@vger.kernel.org> # v6.8 > Signed-off-by: Miklos Szeredi <mszeredi@redhat.com> > --- > 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@kernel.org>
On Thu, 30 Jan 2025 13:15:00 +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). > > [...] Applied to the vfs.fixes branch of the vfs/vfs.git tree. Patches in the vfs.fixes branch should appear in linux-next soon. Please report any outstanding bugs that were missed during review in a new review to the original patch series allowing us to drop it. It's encouraged to provide Acked-bys and Reviewed-bys even though the patch has now been applied. If possible patch trailers will be updated. Note that commit hashes shown below are subject to change due to rebase, trailer updates or similar. If in doubt, please check the listed branch. tree: https://git.kernel.org/pub/scm/linux/kernel/git/vfs/vfs.git branch: vfs.fixes [1/1] statmount: let unset strings be empty https://git.kernel.org/vfs/vfs/c/756060a7cc55
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; }
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@mail.gmail.com/ Fixes: 68385d77c05b ("statmount: simplify string option retrieval") Fixes: 46eae99ef733 ("add statmount(2) syscall") Cc: <stable@vger.kernel.org> # v6.8 Signed-off-by: Miklos Szeredi <mszeredi@redhat.com> --- fs/namespace.c | 25 ++++++++++++++++--------- 1 file changed, 16 insertions(+), 9 deletions(-)