blob: 8d6084239ebcffe677433f23cd1641cc5f34f91e [file] [log] [blame]
Patrick Williams03514f12024-04-05 07:04:11 -05001From 4842cff4f1329f0b5034b529d56f8ad1f234ac4c Mon Sep 17 00:00:00 2001
Andrew Geissler595f6302022-01-24 19:11:47 +00002From: Andre McCurdy <armccurdy@gmail.com>
3Date: Tue, 10 Oct 2017 14:33:30 -0700
Patrick Williams03514f12024-04-05 07:04:11 -05004Subject: [PATCH 07/22] don't pass AT_SYMLINK_NOFOLLOW flag to faccessat()
Andrew Geissler595f6302022-01-24 19:11:47 +00005
6Avoid using AT_SYMLINK_NOFOLLOW flag. It doesn't seem like the right
7thing to do and it's not portable (not supported by musl). See:
8
9 http://lists.landley.net/pipermail/toybox-landley.net/2014-September/003610.html
10 http://www.openwall.com/lists/musl/2015/02/05/2
11
12Note that laccess() is never passing AT_EACCESS so a lot of the
13discussion in the links above doesn't apply. Note also that
14(currently) all systemd callers of laccess() pass mode as F_OK, so
15only check for existence of a file, not access permissions.
16Therefore, in this case, the only distiction between faccessat()
17with (flag == 0) and (flag == AT_SYMLINK_NOFOLLOW) is the behaviour
18for broken symlinks; laccess() on a broken symlink will succeed with
19(flag == AT_SYMLINK_NOFOLLOW) and fail (flag == 0).
20
21The laccess() macros was added to systemd some time ago and it's not
22clear if or why it needs to return success for broken symlinks. Maybe
23just historical and not actually necessary or desired behaviour?
24
25Upstream-Status: Inappropriate [musl specific]
26
27Signed-off-by: Andre McCurdy <armccurdy@gmail.com>
Andrew Geissler595f6302022-01-24 19:11:47 +000028---
Patrick Williamsda295312023-12-05 16:48:56 -060029 src/basic/fs-util.h | 21 ++++++++++++++++++++-
Andrew Geissler595f6302022-01-24 19:11:47 +000030 src/shared/base-filesystem.c | 6 +++---
Patrick Williamsda295312023-12-05 16:48:56 -060031 2 files changed, 23 insertions(+), 4 deletions(-)
Andrew Geissler595f6302022-01-24 19:11:47 +000032
Patrick Williamsda295312023-12-05 16:48:56 -060033diff --git a/src/basic/fs-util.h b/src/basic/fs-util.h
Patrick Williams03514f12024-04-05 07:04:11 -050034index 1023ab73ca..c78ff6f27f 100644
Andrew Geissler595f6302022-01-24 19:11:47 +000035--- a/src/basic/fs-util.h
36+++ b/src/basic/fs-util.h
Patrick Williamsda295312023-12-05 16:48:56 -060037@@ -49,8 +49,27 @@ int futimens_opath(int fd, const struct timespec ts[2]);
Andrew Geissler595f6302022-01-24 19:11:47 +000038 int fd_warn_permissions(const char *path, int fd);
39 int stat_warn_permissions(const char *path, const struct stat *st);
40
41+/*
42+ Avoid using AT_SYMLINK_NOFOLLOW flag. It doesn't seem like the right thing to
43+ do and it's not portable (not supported by musl). See:
44+
45+ http://lists.landley.net/pipermail/toybox-landley.net/2014-September/003610.html
46+ http://www.openwall.com/lists/musl/2015/02/05/2
47+
48+ Note that laccess() is never passing AT_EACCESS so a lot of the discussion in
49+ the links above doesn't apply. Note also that (currently) all systemd callers
50+ of laccess() pass mode as F_OK, so only check for existence of a file, not
51+ access permissions. Therefore, in this case, the only distiction between
52+ faccessat() with (flag == 0) and (flag == AT_SYMLINK_NOFOLLOW) is the
53+ behaviour for broken symlinks; laccess() on a broken symlink will succeed
54+ with (flag == AT_SYMLINK_NOFOLLOW) and fail (flag == 0).
55+
56+ The laccess() macros was added to systemd some time ago and it's not clear if
57+ or why it needs to return success for broken symlinks. Maybe just historical
58+ and not actually necessary or desired behaviour?
59+*/
60 #define laccess(path, mode) \
61- RET_NERRNO(faccessat(AT_FDCWD, (path), (mode), AT_SYMLINK_NOFOLLOW))
62+ RET_NERRNO(faccessat(AT_FDCWD, (path), (mode), 0))
63
64 int touch_file(const char *path, bool parents, usec_t stamp, uid_t uid, gid_t gid, mode_t mode);
Patrick Williamsda295312023-12-05 16:48:56 -060065
66diff --git a/src/shared/base-filesystem.c b/src/shared/base-filesystem.c
Patrick Williams03514f12024-04-05 07:04:11 -050067index 569ef466c3..7ae921a113 100644
Andrew Geissler595f6302022-01-24 19:11:47 +000068--- a/src/shared/base-filesystem.c
69+++ b/src/shared/base-filesystem.c
Patrick Williamsda295312023-12-05 16:48:56 -060070@@ -145,7 +145,7 @@ int base_filesystem_create_fd(int fd, const char *root, uid_t uid, gid_t gid) {
71 /* The "root" parameter is decoration only – it's only used as part of log messages */
Andrew Geissler595f6302022-01-24 19:11:47 +000072
73 for (size_t i = 0; i < ELEMENTSOF(table); i++) {
74- if (faccessat(fd, table[i].dir, F_OK, AT_SYMLINK_NOFOLLOW) >= 0)
75+ if (faccessat(fd, table[i].dir, F_OK, 0) >= 0)
76 continue;
77
Patrick Williamsda295312023-12-05 16:48:56 -060078 if (table[i].target) { /* Create as symlink? */
79@@ -153,7 +153,7 @@ int base_filesystem_create_fd(int fd, const char *root, uid_t uid, gid_t gid) {
Andrew Geissler595f6302022-01-24 19:11:47 +000080
81 /* check if one of the targets exists */
82 NULSTR_FOREACH(s, table[i].target) {
83- if (faccessat(fd, s, F_OK, AT_SYMLINK_NOFOLLOW) < 0)
84+ if (faccessat(fd, s, F_OK, 0) < 0)
85 continue;
86
87 /* check if a specific file exists at the target path */
Patrick Williamsda295312023-12-05 16:48:56 -060088@@ -164,7 +164,7 @@ int base_filesystem_create_fd(int fd, const char *root, uid_t uid, gid_t gid) {
Andrew Geissler595f6302022-01-24 19:11:47 +000089 if (!p)
90 return log_oom();
91
92- if (faccessat(fd, p, F_OK, AT_SYMLINK_NOFOLLOW) < 0)
93+ if (faccessat(fd, p, F_OK, 0) < 0)
94 continue;
95 }
96
Patrick Williamsda295312023-12-05 16:48:56 -060097--
Patrick Williams03514f12024-04-05 07:04:11 -0500982.34.1
Patrick Williamsda295312023-12-05 16:48:56 -060099