APPLIED[U]: [SRU][K/L/Unstable][PATCH 1/1] UBUNTU: SAUCE: overlayfs: handle idmapped mounts in ovl_do_(set|remove)xattr

[Andrea Righi]

On Fri, Mar 03, 2023 at 09:19:55AM +0100, Aleksandr Mikhalitsyn wrote:
> On Fri, Mar 3, 2023 at 8:08 AM Andrea Righi <andrea.righi at canonical.com> wrote:
> >
> > On Thu, Mar 02, 2023 at 10:23:55PM +0100, Alexander Mikhalitsyn wrote:
> > > BugLink: http://bugs.launchpad.net/bugs/2009065
> > >
> > > We have to use ovl_upper_mnt_userns(ofs) helper to get proper user namespace
> > > for idmapped layer. Otherwise we'll get -EPERM.
> > >
> > > Right now, overlayfs on top of idmapped layer always mounted as read-only.
> > > This is serious blocker for LXD/LXC unprivileged containers users who run
> > > Docker containers inside.
> > >
> > > Reproducer:
> > > $ cd /idmapped/mount/path
> > > $ mkdir {work,upper,lower,ovl}
> > > $ mount -t overlay overlay -o lowerdir=lower,upperdir=upper,workdir=work ovl
> > > $ touch ovl/test
> > > touch: cannot touch 'ovl/test': Read-only file system
> > >
> > > Error from dmesg:
> > > overlayfs: failed to create directory work/work (errno: 1); mounting read-only
> > >
> > > Reproducible on all Ubuntu kernels with the base >= 5.19
> > >
> > > Fixes: eea996a46f ("UBUNTU: SAUCE: overlayfs: Skip permission checking for trusted.overlayfs.* xattrs")
> > > Signed-off-by: Alexander Mikhalitsyn <aleksandr.mikhalitsyn at canonical.com>
> >
> > It all makes sense to me, we also have a clear reproducer of the
> > problem, so: applied to lunar/linux-unstable (no need to apply this in
> > lunar/linux, because it'll be replaced by linux-unstable in the next
> > respin). Also:
> 
> Great, thanks! What about kinetic/master-next and jammy/hwe-5.19,
> those branches inherit this change somehow?

No, this patch needs to be applied also to kinetic, then hwe-5.19 will
automatically inherit the change.

I already sent my Ack, if we get another one we can apply it to kinetic
as well.

> 
> >
> > Acked-by: Andrea Righi <andrea.righi at canonical.com>
> >
> > BTW, I'm wondering if we have similar cases with our custom overlayfs
> > changes where we just use init_user_ns, instead of the proper namespace.
> 
> Yep, that's really complex stuff. If you check the actual
> torvalds/linux tree fs/overlayfs
> you find like 6 places where init_user_ns is used (and user properly),
> sometimes it is used implicitly through "nop_mnt_idmap"
> [ https://github.com/torvalds/linux/commit/256c8aed2b420a7c57ed6469fbb0f8310f5aeec9
> ]
> So we need to check each case very carefully.

Makes sense, thanks for the clarification.

> 
> > However, if there are similar cases we should be able to catch them
> > running the typical LXD/LXC test cases I guess.
> 
> Yep, probably we need to add overlayfs to LXC tests somehow to catch
> issues like this.

That'd be great.

> 
> >
> > Do you think we need to add other specific overlayfs test cases to our
> > regression test suite?
> 
> Right now I have no extra tests suggestions.
> 
> Kind regards,
> Alex

Thanks Alex.

-Andrea