All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Andre McCurdy" <armccurdy@gmail.com>
To: Luca Bocassi <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: Thu, 29 Jul 2021 12:11:21 -0700	[thread overview]
Message-ID: <CAJ86T=WgmhDG1OpB3CVf9EtRtxA0hFimKmXod4tcTX3NX1j54w@mail.gmail.com> (raw)
In-Reply-To: <423aed88f51b09d6342089f4b02dc9d62bbdb413.camel@gmail.com>

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.

  parent reply	other threads:[~2021-07-29 19:11 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     ` Andre McCurdy [this message]
2021-08-06 14:12       ` [OE-core] " Luca Bocassi
2021-08-06 18:12         ` Andre McCurdy

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=WgmhDG1OpB3CVf9EtRtxA0hFimKmXod4tcTX3NX1j54w@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.