Merge pull request #27536 from dtardon/checked-fd-parsing

Always check parsed fds for validity
This commit is contained in:
Luca Boccassi 2023-05-05 20:55:48 +01:00 committed by GitHub
commit 6ad7989ea0
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
14 changed files with 75 additions and 47 deletions

View file

@ -416,7 +416,8 @@ int close_all_fds(const int except[], size_t n_except) {
if (!IN_SET(de->d_type, DT_LNK, DT_UNKNOWN))
continue;
if (safe_atoi(de->d_name, &fd) < 0)
fd = parse_fd(de->d_name);
if (fd < 0)
/* Let's better ignore this, just in case */
continue;

View file

@ -333,6 +333,21 @@ int parse_errno(const char *t) {
return e;
}
int parse_fd(const char *t) {
int r, fd;
assert(t);
r = safe_atoi(t, &fd);
if (r < 0)
return r;
if (fd < 0)
return -ERANGE;
return fd;
}
static const char *mangle_base(const char *s, unsigned *base) {
const char *k;

View file

@ -21,6 +21,7 @@ int parse_size(const char *t, uint64_t base, uint64_t *size);
int parse_sector_size(const char *t, uint64_t *ret);
int parse_range(const char *t, unsigned *lower, unsigned *upper);
int parse_errno(const char *t);
int parse_fd(const char *t);
#define SAFE_ATO_REFUSE_PLUS_MINUS (1U << 30)
#define SAFE_ATO_REFUSE_LEADING_ZERO (1U << 29)

View file

@ -938,7 +938,7 @@ static int automount_deserialize_item(Unit *u, const char *key, const char *valu
} else if (streq(key, "pipe-fd")) {
int fd;
if (safe_atoi(value, &fd) < 0 || fd < 0 || !fdset_contains(fds, fd))
if ((fd = parse_fd(value)) < 0 || !fdset_contains(fds, fd))
log_unit_debug(u, "Failed to parse pipe-fd value: %s", value);
else {
safe_close(a->pipe_fd);

View file

@ -645,12 +645,12 @@ void dynamic_user_deserialize_one(Manager *m, const char *value, FDSet *fds) {
return;
}
if (safe_atoi(s0, &fd0) < 0 || !fdset_contains(fds, fd0)) {
if ((fd0 = parse_fd(s0)) < 0 || !fdset_contains(fds, fd0)) {
log_debug("Unable to process dynamic user fd specification.");
return;
}
if (safe_atoi(s1, &fd1) < 0 || !fdset_contains(fds, fd1)) {
if ((fd1 = parse_fd(s1)) < 0 || !fdset_contains(fds, fd1)) {
log_debug("Unable to process dynamic user fd specification.");
return;
}

View file

@ -7338,7 +7338,7 @@ int exec_shared_runtime_deserialize_compat(Unit *u, const char *key, const char
} else if (streq(key, "netns-socket-0")) {
int fd;
if (safe_atoi(value, &fd) < 0 || !fdset_contains(fds, fd)) {
if ((fd = parse_fd(value)) < 0 || !fdset_contains(fds, fd)) {
log_unit_debug(u, "Failed to parse netns socket value: %s", value);
return 0;
}
@ -7349,7 +7349,7 @@ int exec_shared_runtime_deserialize_compat(Unit *u, const char *key, const char
} else if (streq(key, "netns-socket-1")) {
int fd;
if (safe_atoi(value, &fd) < 0 || !fdset_contains(fds, fd)) {
if ((fd = parse_fd(value)) < 0 || !fdset_contains(fds, fd)) {
log_unit_debug(u, "Failed to parse netns socket value: %s", value);
return 0;
}
@ -7422,9 +7422,9 @@ int exec_shared_runtime_deserialize_one(Manager *m, const char *value, FDSet *fd
n = strcspn(v, " ");
buf = strndupa_safe(v, n);
r = safe_atoi(buf, &netns_fdpair[0]);
if (r < 0)
return log_debug_errno(r, "Unable to parse exec-runtime specification netns-socket-0=%s: %m", buf);
netns_fdpair[0] = parse_fd(buf);
if (netns_fdpair[0] < 0)
return log_debug_errno(netns_fdpair[0], "Unable to parse exec-runtime specification netns-socket-0=%s: %m", buf);
if (!fdset_contains(fds, netns_fdpair[0]))
return log_debug_errno(SYNTHETIC_ERRNO(EBADF),
"exec-runtime specification netns-socket-0= refers to unknown fd %d: %m", netns_fdpair[0]);
@ -7441,9 +7441,9 @@ int exec_shared_runtime_deserialize_one(Manager *m, const char *value, FDSet *fd
n = strcspn(v, " ");
buf = strndupa_safe(v, n);
r = safe_atoi(buf, &netns_fdpair[1]);
if (r < 0)
return log_debug_errno(r, "Unable to parse exec-runtime specification netns-socket-1=%s: %m", buf);
netns_fdpair[1] = parse_fd(buf);
if (netns_fdpair[1] < 0)
return log_debug_errno(netns_fdpair[1], "Unable to parse exec-runtime specification netns-socket-1=%s: %m", buf);
if (!fdset_contains(fds, netns_fdpair[1]))
return log_debug_errno(SYNTHETIC_ERRNO(EBADF),
"exec-runtime specification netns-socket-1= refers to unknown fd %d: %m", netns_fdpair[1]);
@ -7460,9 +7460,9 @@ int exec_shared_runtime_deserialize_one(Manager *m, const char *value, FDSet *fd
n = strcspn(v, " ");
buf = strndupa_safe(v, n);
r = safe_atoi(buf, &ipcns_fdpair[0]);
if (r < 0)
return log_debug_errno(r, "Unable to parse exec-runtime specification ipcns-socket-0=%s: %m", buf);
ipcns_fdpair[0] = parse_fd(buf);
if (ipcns_fdpair[0] < 0)
return log_debug_errno(ipcns_fdpair[0], "Unable to parse exec-runtime specification ipcns-socket-0=%s: %m", buf);
if (!fdset_contains(fds, ipcns_fdpair[0]))
return log_debug_errno(SYNTHETIC_ERRNO(EBADF),
"exec-runtime specification ipcns-socket-0= refers to unknown fd %d: %m", ipcns_fdpair[0]);
@ -7479,9 +7479,9 @@ int exec_shared_runtime_deserialize_one(Manager *m, const char *value, FDSet *fd
n = strcspn(v, " ");
buf = strndupa_safe(v, n);
r = safe_atoi(buf, &ipcns_fdpair[1]);
if (r < 0)
return log_debug_errno(r, "Unable to parse exec-runtime specification ipcns-socket-1=%s: %m", buf);
ipcns_fdpair[1] = parse_fd(buf);
if (ipcns_fdpair[1] < 0)
return log_debug_errno(ipcns_fdpair[1], "Unable to parse exec-runtime specification ipcns-socket-1=%s: %m", buf);
if (!fdset_contains(fds, ipcns_fdpair[1]))
return log_debug_errno(SYNTHETIC_ERRNO(EBADF),
"exec-runtime specification ipcns-socket-1= refers to unknown fd %d: %m", ipcns_fdpair[1]);

View file

@ -1002,13 +1002,11 @@ static int parse_argv(int argc, char *argv[]) {
int fd;
FILE *f;
r = safe_atoi(optarg, &fd);
if (r < 0)
log_error_errno(r, "Failed to parse deserialize option \"%s\": %m", optarg);
fd = parse_fd(optarg);
if (fd == -ERANGE)
return log_error_errno(fd, "Invalid serialization fd: %s", optarg);
if (fd < 0)
return log_error_errno(SYNTHETIC_ERRNO(EINVAL),
"Invalid deserialize fd: %d",
fd);
return log_error_errno(fd, "Failed to parse deserialize option \"%s\": %m", optarg);
(void) fd_cloexec(fd, true);

View file

@ -476,7 +476,7 @@ int manager_deserialize(Manager *m, FILE *f, FDSet *fds) {
} else if ((val = startswith(l, "notify-fd="))) {
int fd;
if (safe_atoi(val, &fd) < 0 || fd < 0 || !fdset_contains(fds, fd))
if ((fd = parse_fd(val)) < 0 || !fdset_contains(fds, fd))
log_notice("Failed to parse notify fd, ignoring: \"%s\"", val);
else {
m->notify_event_source = sd_event_source_disable_unref(m->notify_event_source);
@ -492,7 +492,7 @@ int manager_deserialize(Manager *m, FILE *f, FDSet *fds) {
} else if ((val = startswith(l, "cgroups-agent-fd="))) {
int fd;
if (safe_atoi(val, &fd) < 0 || fd < 0 || !fdset_contains(fds, fd))
if ((fd = parse_fd(val)) < 0 || !fdset_contains(fds, fd))
log_notice("Failed to parse cgroups agent fd, ignoring.: %s", val);
else {
m->cgroups_agent_event_source = sd_event_source_disable_unref(m->cgroups_agent_event_source);

View file

@ -3235,7 +3235,7 @@ static int service_deserialize_item(Unit *u, const char *key, const char *value,
} else if (streq(key, "socket-fd")) {
int fd;
if (safe_atoi(value, &fd) < 0 || fd < 0 || !fdset_contains(fds, fd))
if ((fd = parse_fd(value)) < 0 || !fdset_contains(fds, fd))
log_unit_debug(u, "Failed to parse socket-fd value: %s", value);
else {
asynchronous_close(s->socket_fd);
@ -3246,7 +3246,7 @@ static int service_deserialize_item(Unit *u, const char *key, const char *value,
int fd, do_poll;
r = extract_first_word(&value, &fdv, NULL, 0);
if (r <= 0 || safe_atoi(fdv, &fd) < 0 || fd < 0 || !fdset_contains(fds, fd)) {
if (r <= 0 || (fd = parse_fd(fdv)) < 0 || !fdset_contains(fds, fd)) {
log_unit_debug(u, "Failed to parse fd-store-fd value: %s", value);
return 0;
}
@ -3324,7 +3324,7 @@ static int service_deserialize_item(Unit *u, const char *key, const char *value,
} else if (streq(key, "stdin-fd")) {
int fd;
if (safe_atoi(value, &fd) < 0 || fd < 0 || !fdset_contains(fds, fd))
if ((fd = parse_fd(value)) < 0 || !fdset_contains(fds, fd))
log_unit_debug(u, "Failed to parse stdin-fd value: %s", value);
else {
asynchronous_close(s->stdin_fd);
@ -3334,7 +3334,7 @@ static int service_deserialize_item(Unit *u, const char *key, const char *value,
} else if (streq(key, "stdout-fd")) {
int fd;
if (safe_atoi(value, &fd) < 0 || fd < 0 || !fdset_contains(fds, fd))
if ((fd = parse_fd(value)) < 0 || !fdset_contains(fds, fd))
log_unit_debug(u, "Failed to parse stdout-fd value: %s", value);
else {
asynchronous_close(s->stdout_fd);
@ -3344,7 +3344,7 @@ static int service_deserialize_item(Unit *u, const char *key, const char *value,
} else if (streq(key, "stderr-fd")) {
int fd;
if (safe_atoi(value, &fd) < 0 || fd < 0 || !fdset_contains(fds, fd))
if ((fd = parse_fd(value)) < 0 || !fdset_contains(fds, fd))
log_unit_debug(u, "Failed to parse stderr-fd value: %s", value);
else {
asynchronous_close(s->stderr_fd);
@ -3354,7 +3354,7 @@ static int service_deserialize_item(Unit *u, const char *key, const char *value,
} else if (streq(key, "exec-fd")) {
int fd;
if (safe_atoi(value, &fd) < 0 || fd < 0 || !fdset_contains(fds, fd))
if ((fd = parse_fd(value)) < 0 || !fdset_contains(fds, fd))
log_unit_debug(u, "Failed to parse exec-fd value: %s", value);
else {
s->exec_fd_event_source = sd_event_source_disable_unref(s->exec_fd_event_source);

View file

@ -391,7 +391,7 @@ int unit_deserialize(Unit *u, FILE *f, FDSet *fds) {
else if (STR_IN_SET(l, "ipv4-socket-bind-bpf-link-fd", "ipv6-socket-bind-bpf-link-fd")) {
int fd;
if (safe_atoi(v, &fd) < 0 || fd < 0 || !fdset_contains(fds, fd))
if ((fd = parse_fd(v)) < 0 || !fdset_contains(fds, fd))
log_unit_debug(u, "Failed to parse %s value: %s, ignoring.", l, v);
else {
if (fdset_remove(fds, fd) < 0) {
@ -423,7 +423,7 @@ int unit_deserialize(Unit *u, FILE *f, FDSet *fds) {
} else if (streq(l, "restrict-ifaces-bpf-fd")) {
int fd;
if (safe_atoi(v, &fd) < 0 || fd < 0 || !fdset_contains(fds, fd)) {
if ((fd = parse_fd(v)) < 0 || !fdset_contains(fds, fd)) {
log_unit_debug(u, "Failed to parse restrict-ifaces-bpf-fd value: %s", v);
continue;
}

View file

@ -215,11 +215,11 @@ static int parse_argv(int argc, char *argv[]) {
_cleanup_close_ int owned_fd = -EBADF;
int fdnr;
r = safe_atoi(optarg, &fdnr);
if (r < 0)
return log_error_errno(r, "Failed to parse file descriptor: %s", optarg);
fdnr = parse_fd(optarg);
if (fdnr == -ERANGE)
return log_error_errno(fdnr, "File descriptor can't be negative: %s", optarg);
if (fdnr < 0)
return log_error_errno(SYNTHETIC_ERRNO(ERANGE), "File descriptor can't be negative: %i", fdnr);
return log_error_errno(fdnr, "Failed to parse file descriptor: %s", optarg);
if (!passed) {
/* Take possession of all passed fds */

View file

@ -449,11 +449,11 @@ int bpf_program_deserialize_attachment(const char *v, FDSet *fds, BPFProgram **b
if (r == 0)
return -EINVAL;
r = safe_atoi(sfd, &ifd);
if (r < 0)
return r;
if (ifd < 0)
ifd = parse_fd(sfd);
if (ifd == -ERANGE)
return -EBADF;
if (ifd < 0)
return r;
/* Extract second word: the attach type */
r = extract_first_word(&v, &sat, NULL, 0);

View file

@ -3060,12 +3060,12 @@ int varlink_server_deserialize_one(VarlinkServer *s, const char *value, FDSet *f
n = strcspn(v, " ");
buf = strndupa_safe(v, n);
r = safe_atoi(buf, &fd);
if (r < 0)
return log_debug_errno(r, "Unable to parse VarlinkServerSocket varlink-server-socket-fd=%s: %m", buf);
if (fd < 0)
fd = parse_fd(buf);
if (fd == -ERANGE)
return log_debug_errno(SYNTHETIC_ERRNO(EINVAL),
"VarlinkServerSocket varlink-server-socket-fd= has an invalid value: %d", fd);
"VarlinkServerSocket varlink-server-socket-fd= has an invalid value: %s", buf);
if (fd < 0)
return log_debug_errno(fd, "Unable to parse VarlinkServerSocket varlink-server-socket-fd=%s: %m", buf);
if (!fdset_contains(fds, fd))
return log_debug_errno(SYNTHETIC_ERRNO(EBADF),
"VarlinkServerSocket varlink-server-socket-fd= has unknown fd %d: %m", fd);

View file

@ -865,6 +865,19 @@ TEST(parse_errno) {
assert_se(parse_errno("EINVALaaa") == -EINVAL);
}
TEST(parse_fd) {
assert_se(parse_fd("0") == 0);
assert_se(parse_fd("1") == 1);
assert_se(parse_fd("-1") == -ERANGE);
assert_se(parse_fd("-3") == -ERANGE);
assert_se(parse_fd("") == -EINVAL);
assert_se(parse_fd("12.3") == -EINVAL);
assert_se(parse_fd("123junk") == -EINVAL);
assert_se(parse_fd("junk123") == -EINVAL);
}
TEST(parse_mtu) {
uint32_t mtu = 0;