Re: [PATCH] attr: block mode changes of symlinks

[Giuseppe Scrivano <giuseppe@scrivano.org>]

Christian Brauner <brauner@kernel.org> writes:

> On Fri, Oct 20, 2023 at 10:34:36AM +0200, Linux regression tracking (Thorsten Leemhuis) wrote:
>> [adding Christian, the author of what appears to be the culprit]
>> 
>> On 18.10.23 20:49, Jesse Hathaway wrote:
>> > On Wed, Oct 18, 2023 at 1:40 PM Greg KH <gregkh@linuxfoundation.org> wrote:
>> 
>> FWIW, this thread afaics was supposed to be in reply to this submission:
>> 
>> https://lore.kernel.org/all/20230712-vfs-chmod-symlinks-v1-1-27921df6011f@kernel.org/
>> 
>> That patch later became 5d1f903f75a80d ("attr: block mode changes of
>> symlinks") [v6.6-rc1, v6.5.5, v6.1.55, v5.4.257, v5.15.133, v5.10.197,
>> v4.19.295, v4.14.326]
>> 
>> >>> Unfortunately, this has not held up in LTSes without causing
>> >>> regressions, specifically in crun:
>> >>>
>> >>> Crun issue and patch
>> >>>  1. https://github.com/containers/crun/issues/1308
>> >>>  2. https://github.com/containers/crun/pull/1309
>> >>
>> >> So thre's a fix already for this, they agree that symlinks shouldn't
>> >> have modes, so what's the issue?
>> > 
>> > The problem is that it breaks crun in Debian stable. They have fixed the
>> > issue in crun, but that patch may not be backported to Debian's stable
>> > version. In other words the patch seems to break existing software in
>> > the wild.
>> > 
>> >> It needs to reverted in Linus's tree first, otherwise you will hit the
>> >> same problem when moving to a new kernel.
>> > 
>> > Okay, I'll raise the issue on the linux kernel mailing list.
>> 
>> Did you do that? I could not find anything. Just wondering, as right now
>> there is still some time to fix this regression before 6.6 is released
>> (and then the fix can be backported to the stable trees, too).
>
> I have not seen a report other than the crun fix I commented on.
>
> The crun authors had agreed to fix this in crun. As symlink mode changes
> are severly broken to the point that it's not even supported through the
> official glibc and musl system call wrappers anymore not having to
> revert this from mainline would be the ideal outcome.
>
> So ideally, the crun bugfix would be backported to Debian stable just as
> it was already backported to Fedora or crun make a new point release for
> the 1.8.* series.
>
> The other option to consider would be to revert the backport of the attr
> changes to stable kernels. I'm not sure what Greg's stance on this is
> but given that crun versions in -testing already include that fix that
> means all future Debian releases will already have a fixed crun version.
>
> That symlink stuff is so brittle and broken that we'd do more long-term
> harm by letting it go on. Which is why we did this.
>
> @Linus, this is ultimately your call of course.

my two cents as the crun maintainer:

We were messing with /proc/*/fd files to do something not supported.
The kernel patch made the error explicit instead of ignoring errors just
in some cases.

Since it was already fixed upstream in crun and the fix is included in
the last three releases, Debian could simply pick a newer version; or I
can help with a backport if that is what they prefer.