Re: [regression?] escaping commas in overlayfs mount options

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Tue, Oct 10, 2023 at 12:06 PM Miklos Szeredi <miklos@xxxxxxxxxx> wrote:
>
> On Fri, 6 Oct 2023 at 19:21, Amir Goldstein <amir73il@xxxxxxxxx> wrote:
> >
> > On Fri, Oct 6, 2023 at 7:42 PM Ryan Hendrickson
>
> > > And there is the escaping that needs to happen for ':' and '\' when
> > > parsing the path parameters (':' is only special syntax in lowerdir, but
> > > the escaping logic seems to apply to upperdir and workdir as well, based
> > > on my testing). Even using the new API, this is handled in the kernel.
> > > We'd like to know if this escaping can be considered stable as well, and I
> > > don't think that's a question for the libmount maintainer.
> >
> > Agree.
> > Unlike the comma separated parameters list,
> > upperdir,workdir,lowerdir are overlayfs specific format.
> >
> > ovl_unescape() (for upperdir/workdir) unescapes '\' characters.
> > as does ovl_parse_param_split_lowerdirs().
> > Not sure why this was needed for upperdir/workdir, but it It has
> > been this way for a long time.
> > I see no reason for it to change in the future.
>
> Unescaping  upperdir/workdir was the side effect of using a common
> helper; it wasn't intentional, I think.  The problem is that
> unescaping breaks code that doesn't expect it, and filenames with
> backslashes (and especially \\ or \: sequences) are very rare, so this
> won't show up in testing.
>
> At this point I'm not sure which is more likely to cause bugs: getting
> rid of unescaping or leaving it alone.

Considering the fact that the applications that mount overlayfs has
always had to do the correct escaping, getting rid of escaping can
only solve issues in new deployments, so I think we should greatly
favor leaving it alone.

>
> One way out of this mess is to create explicit _unesc versions of these options.
>

I like that solution, with two reservations:
1. IMO, new _unesc versions should only be supported from new mount API
2. I only want to do that if real users exists - said users are expected
    to send the patch and explain their use case

Thanks,
Amir.




[Index of Archives]     [Linux Filesystems Devel]     [Linux NFS]     [Linux NILFS]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux