All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Andre McCurdy" <armccurdy@gmail.com>
To: Luca Boccassi <luca.boccassi@gmail.com>
Cc: OE-core <openembedded-core@lists.openembedded.org>,
	Khem Raj <raj.khem@gmail.com>
Subject: Re: [OE-core] [PATCH] systemd: Fix build on musl
Date: Fri, 6 Aug 2021 11:12:15 -0700	[thread overview]
Message-ID: <CAJ86T=VQgcOBvFLVL97Oy=m4_jn3-p=Jnir-WLhJSq6y+_wHTQ@mail.gmail.com> (raw)
In-Reply-To: <CAMw=ZnQuXfz81KeW81E_K-1yfpTFZ9x-Ct1gTg8S6CiucBUQ+Q@mail.gmail.com>

On Fri, Aug 6, 2021 at 7:12 AM Luca Boccassi <luca.boccassi@gmail.com> wrote:
>
> On Thu, 29 Jul 2021 at 20:11, Andre McCurdy <armccurdy@gmail.com> wrote:
> >
> > On Thu, Jul 29, 2021 at 6:49 AM Luca Bocassi <luca.boccassi@gmail.com> wrote:
> > >
> > > Having a look at the patches, a few comments:
> > >
> > > - 0012-don-t-pass-AT_SYMLINK_NOFOLLOW-flag-to-faccessat.patch I find
> > > quite worrying, as it fundamentally changes access patterns, some of
> > > which are done for security reasons. At best, this will cause
> > > completely different runtime behaviours for the same filesystem
> > > depending on the libc implementation, which doesn't sound great?
> >
> > I wrote a long and verbose comment when I created the patch which
> > tries to document any differences in runtime behaviour.
> >
> >   ----
> >   Avoid using AT_SYMLINK_NOFOLLOW flag. It doesn't seem like the right thing to
> >   do and it's not portable (not supported by musl). See:
> >
> >     http://lists.landley.net/pipermail/toybox-landley.net/2014-September/003610.html
> >     http://www.openwall.com/lists/musl/2015/02/05/2
> >
> >   Note that laccess() is never passing AT_EACCESS so a lot of the discussion in
> >   the links above doesn't apply. Note also that (currently) all systemd callers
> >   of laccess() pass mode as F_OK, so only check for existence of a file, not
> >   access permissions. Therefore, in this case, the only distiction between
> >   faccessat() with (flag == 0) and (flag == AT_SYMLINK_NOFOLLOW) is the
> >   behaviour for broken symlinks; laccess() on a broken symlink will succeed
> >   with (flag == AT_SYMLINK_NOFOLLOW) and fail (flag == 0).
> >
> >   The laccess() macros was added to systemd some time ago and it's not clear if
> >   or why it needs to return success for broken symlinks. Maybe just historical
> >   and not actually necessary or desired behaviour?
> >   ----
> >
> > If that comment is now out of date or something is missing then please
> > send a patch to update it.
> >
> > However looking at this patch again now, it appears to have got broken
> > during a past rebase:
> >
> >   https://git.openembedded.org/openembedded-core/commit/?id=e8dd5a36bf2f1e645fb2ff15eb3b5e97c04776e6
> >
> > The upstream code changed from:
> >
> >   #define laccess(path, mode) faccessat(AT_FDCWD, (path), (mode),
> > AT_SYMLINK_NOFOLLOW)
> >
> > to
> >
> >   #define laccess(path, mode)                                             \
> >           (faccessat(AT_FDCWD, (path), (mode), AT_SYMLINK_NOFOLLOW) <
> > 0 ? -errno : 0)
> >
> > but the replacement version in the patch still returns the raw result
> > from faccessat(). That looks like an issue.
>
> If you think the flag is unnecessary (I don't, we use these for a
> reason, but that's not important right now), the correct action is to
> send a PR upstream to discuss removing it. Patching it out for one
> build case of many is just going to be a source of incompatibility and
> surprises for users, as the behaviour on the same filesystem changes
> depending on the build option. Having said that, I don't use musl so
> all of this is really not a problem for me, just providing some
> feedback as upstream maintainer, in case it can be useful.

I don't have any interest in systemd + musl anymore either. I did an
initial port as a proof of concept and sent patches to Khem off
list... and was somewhat surprised when they showed up some time later
in oe-core.

Note that there have been long discussions here previously about
whether OE should claim to support systemd + musl. The effort to
support it properly (including clarifying questions like this with
upstream as you suggest) doesn't seem huge but so far no one seems to
care enough about systemd + musl to do it. We rebase and tweak the
patches but guidance to potential users should still be "use at your
own risk".

      reply	other threads:[~2021-08-06 18:12 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-07-27 20:13 Khem Raj
2021-07-29 12:55 ` [OE-core] " Robert Berger
2021-07-29 16:00   ` Khem Raj
     [not found] ` <AM7PR83MB0436960F9D4BFDD1F504F0DFF1EB9@AM7PR83MB0436.EURPRD83.prod.outlook.com>
2021-07-29 13:49   ` Luca Bocassi
2021-07-29 13:54     ` Luca Bocassi
2021-07-29 14:37       ` [OE-core] " Alexander Kanavin
2021-07-29 17:47       ` Khem Raj
2021-07-29 19:11     ` [OE-core] " Andre McCurdy
2021-08-06 14:12       ` Luca Bocassi
2021-08-06 18:12         ` Andre McCurdy [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to='CAJ86T=VQgcOBvFLVL97Oy=m4_jn3-p=Jnir-WLhJSq6y+_wHTQ@mail.gmail.com' \
    --to=armccurdy@gmail.com \
    --cc=luca.boccassi@gmail.com \
    --cc=openembedded-core@lists.openembedded.org \
    --cc=raj.khem@gmail.com \
    --subject='Re: [OE-core] [PATCH] systemd: Fix build on musl' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.