| Message ID | 20250319214900.25100-1-kernel@bareminimum.eu (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | btrfs: correctly escape subvol in btrfs_show_options | expand |
On Wed, Mar 19, 2025 at 10:49:00PM +0100, Johannes Kimmel wrote: > Currently, displaying the btrfs subvol mount option doesn't escape `,`. > This makes parsing /proc/self/mounts and /proc/self/mountinfo > ambigious for subvolume names that contain commas. The text after the > comma could be mistaken for another option (think "subvol=foo,ro", where > ro is actually part of the subvolumes name). The subvol= option was intentionally last so the path does not mix with other options but yeah it still can be confused if it's parsed in a generic way and not assuming anything about the ordering. I've checked util-linux/libmount, seems that it understands the "\," as part of the option value so it should not break anything. > This patch replaces the manual escape characters list with a call to > seq_show_option. Thanks to Calvin Walton for suggesting this approach. > > Fixes: c8d3fe028f64 ("Btrfs: show subvol= and subvolid= in /proc/mounts") > Suggested-by: Calvin Walton <calvin.walton@kepstin.ca> > Signed-off-by: Johannes Kimmel <kernel@bareminimum.eu> Added to for-next, thanks.
On 20.03.25 00:52, David Sterba wrote: > On Wed, Mar 19, 2025 at 10:49:00PM +0100, Johannes Kimmel wrote: >> Currently, displaying the btrfs subvol mount option doesn't escape `,`. >> This makes parsing /proc/self/mounts and /proc/self/mountinfo >> ambigious for subvolume names that contain commas. The text after the >> comma could be mistaken for another option (think "subvol=foo,ro", where >> ro is actually part of the subvolumes name). > > The subvol= option was intentionally last so the path does not mix with > other options but yeah it still can be confused if it's parsed in a > generic way and not assuming anything about the ordering. > > I've checked util-linux/libmount, seems that it understands the "\," as > part of the option value so it should not break anything. > >> This patch replaces the manual escape characters list with a call to >> seq_show_option. Thanks to Calvin Walton for suggesting this approach. >> >> Fixes: c8d3fe028f64 ("Btrfs: show subvol= and subvolid= in /proc/mounts") >> Suggested-by: Calvin Walton <calvin.walton@kepstin.ca> >> Signed-off-by: Johannes Kimmel <kernel@bareminimum.eu> > > Added to for-next, thanks. Thank you for taking care of this so quickly.
diff --git a/fs/btrfs/super.c b/fs/btrfs/super.c index dc4fee519ca6..a5f29ff3fbc2 100644 --- a/fs/btrfs/super.c +++ b/fs/btrfs/super.c @@ -1139,8 +1139,7 @@ static int btrfs_show_options(struct seq_file *seq, struct dentry *dentry) subvol_name = btrfs_get_subvol_name_from_objectid(info, btrfs_root_id(BTRFS_I(d_inode(dentry))->root)); if (!IS_ERR(subvol_name)) { - seq_puts(seq, ",subvol="); - seq_escape(seq, subvol_name, " \t\n\\"); + seq_show_option(seq, "subvol", subvol_name); kfree(subvol_name); } return 0;
Currently, displaying the btrfs subvol mount option doesn't escape `,`. This makes parsing /proc/self/mounts and /proc/self/mountinfo ambigious for subvolume names that contain commas. The text after the comma could be mistaken for another option (think "subvol=foo,ro", where ro is actually part of the subvolumes name). This patch replaces the manual escape characters list with a call to seq_show_option. Thanks to Calvin Walton for suggesting this approach. Fixes: c8d3fe028f64 ("Btrfs: show subvol= and subvolid= in /proc/mounts") Suggested-by: Calvin Walton <calvin.walton@kepstin.ca> Signed-off-by: Johannes Kimmel <kernel@bareminimum.eu> --- fs/btrfs/super.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-)