Re: two questiones about overlayfs

[Vivek Goyal <vgoyal@redhat.com>]

On Tue, Aug 15, 2017 at 05:33:01PM +0200, Miklos Szeredi wrote:
> On Tue, Aug 15, 2017 at 5:28 PM, Amir Goldstein <amir73il@gmail.com> wrote:
> > On Tue, Aug 15, 2017 at 5:06 PM, Miklos Szeredi <miklos@szeredi.hu> wrote:
> >> On Tue, Aug 15, 2017 at 4:52 PM, Amir Goldstein <amir73il@gmail.com> wrote:
> >>> On Tue, Aug 15, 2017 at 3:35 PM, Miklos Szeredi <miklos@szeredi.hu> wrote:
> >>>> On Tue, Aug 08, 2017 at 07:01:30AM +0200, Amir Goldstein wrote:
> >>>>> On Mon, Aug 7, 2017 at 9:57 AM, zhangyi (F) <yi.zhang@huawei.com> wrote:
> >>>>
> >>>> [snip]
> >>>>
> >>>>> > 2. Chattr will modify lower file's attributes directly.
> >>>>> > Reproduce:
> >>>>> > # mkdir lower upper worker merger
> >>>>> > # touch lower/aa
> >>>>> > # lsattr -p lower/aa
> >>>>> >     0 --------------e---- lower/aa
> >>>>> > # mount -t overlay -o lowerdir=lower,upperdir=upper,workdir=worker overlayfs merger
> >>>>> > # chattr -p 123 merger/aa             #set project id
> >>>>> > # lsattr -p lower/aa
> >>>>> >   123 --------------e---- lower/aa
> >>>>> >
> >>>>> > If we try to set "immutable" or any other attributes, the result are consistent.
> >>>>> > Because chattr open file in RDONLY mode, so it will not trigger copyup, and then,
> >>>>> > FS_IOC_SETFLAGS ioctl will get the lower inode and modify it.
> >>>>>
> >>>>> Ouch! I guess it's a "known to some" issue.
> >>>>> Fixing this would be a pain (intercept ioctl and whitelisting readonly
> >>>>> fs specific ioctls).
> >>>>
> >>>> Fixing ioctl properly would be a pain.  But we can hack around the issue, and
> >>>> just deny it for now.
> >>>>
> >>>> See patch below
> >>>
> >>> I like this, but it will require good test coverage of fs specific ioctls.
> >>> The list of filesystems that call  mnt_want_write_file() for ioctl is not short.
> >>
> >> If it's called from within the filesystem, then the new behavior is
> >> certainly the correct one.
> >
> > It certainly is. It doesn't mean that fixing incorrect behavior won't
> > lead to unacceptable regressions, which may require explicit
> > d_real() call from filesystem to be fixed.
> 
> I don't get it.  The only possible regression is denying modification
> on lower layer where previously was allowed.  But anybody relying on
> that would be pretty crazy.

Hi Miklos,

IIUC, so now "chattr -p <id>" will fail on overlayfs (assume file has not
been copied up yet).

IOW, on overlayfs, will it be responsibility of user space to make
sure file has been copied up, for chattr operation to succeed? Does that
mean we need to modify chattr to open file for WRITE instead of READ.

Vivek