On Sat, Oct 14, 2023 at 9:20 PM Miklos Szeredi <miklos@xxxxxxxxxx> wrote:
>
> On Sat, 14 Oct 2023 at 19:31, Miklos Szeredi <miklos@xxxxxxxxxx> wrote:
>
> > If you can code this up quickly, that's good. I can have a go at it
> > on Monday, but my PoC patch needs splitting up and so it's not ready
> > for 6.6.
>
> Attaching my current patch (against your 3 patches).
>
> +static char *escape_colons(char *s, char *p)
> +{
> + char *ret = s;
> +
> + for (;;) {
> + char c = *p++;
> + if (c != ':') {
> + *s++ = c;
> + if (!c)
> + return ret;
> + } else if (s + 2 > p) {
> + return ERR_PTR(-ENAMETOOLONG);
> + } else {
> + *s++ = '\\';
> + *s++ = c;
> + }
> + }
> +}
I think it is a bad idea going down this rabbit hole.
This escaping is incorrect for an already escaped input
(e.g. "lowerdir=/a\:b") - it would escape the \ instead of :
I think that playing more escaping games is the opposite of what
the explicit path params want to achieve.
Suggesting instead:
--- a/fs/overlayfs/params.c
+++ b/fs/overlayfs/params.c
@@ -933,23 +933,27 @@ int ovl_show_options(struct seq_file *m, struct
dentry *dentry)
{
struct super_block *sb = dentry->d_sb;
struct ovl_fs *ofs = OVL_FS(sb);
- size_t nr, nr_merged_lower = ofs->numlayer - ofs->numdatalayer;
+ size_t nr, nr_merged_lower, nr_added_lower = 0;
+ const char **lowerdirs = ofs->config.lowerdirs;
/*
- * lowerdirs[] starts from offset 1, then
- * >= 0 regular lower layers prefixed with : and
- * >= 0 data-only lower layers prefixed with ::
- *
- * we need to escase comma and space like seq_show_option() does and
- * we also need to escape the colon separator from lowerdir paths.
+ * lowerdirs[0] holds the colon separated list if user provided
+ * it with legacy lowerdir= mount option. Otherwise, we use the
+ * options lowerdir+ and datadir+ to show the separated lowerdirs
+ * that are stored in lowerdirs[1..numlayer].
*/
- seq_puts(m, ",lowerdir=");
- for (nr = 1; nr < ofs->numlayer; nr++) {
- if (nr > 1)
- seq_putc(m, ':');
- if (nr >= nr_merged_lower)
- seq_putc(m, ':');
- seq_escape(m, ofs->config.lowerdirs[nr], ":, \t\n\\");
+ if (lowerdirs[0]) {
+ seq_show_option(m, "lowerdir", lowerdirs[0]);
+ } else {
+ nr_added_lower = ofs->numlayer - 1;
+ nr_merged_lower = nr_added_lower - ofs->numdatalayer;
+ lowerdirs++;
+ }
+ for (nr = 0; nr < nr_added_lower; nr++, lowerdirs++) {
+ if (nr < nr_merged_lower)
+ seq_show_option(m, "lowerdir+", *lowerdirs);
+ else
+ seq_show_option(m, "datadir+", *lowerdirs);
}
if (ofs->config.upperdir) {
seq_show_option(m, "upperdir", ofs->config.upperdir);
---
This implies that we do not allow mixing legacy lowerdir= with
new lowerdir+/datadir+.
I don't see why mixing would be needed.
Thanks,
Amir.
