notify-recv: several followups

Follow-up for 7f6af95dab

- Allocate internal buf on the stack, memdup() only at the end.
  This ensures we're able to handle OOM gracefully, i.e.
  return -EAGAIN on OOM while still emptying socket buffer.
- Do not treat empty notify message as error.
- Raise log level since all callers log loudly anyway.
This commit is contained in:
Mike Yuan
2025-02-21 15:16:46 +01:00
parent 2006e341d3
commit 74cd56d31b
6 changed files with 49 additions and 39 deletions

View File

@@ -655,7 +655,7 @@ static int manager_on_notify(sd_event_source *s, int fd, uint32_t revents, void
if (r == -EAGAIN)
return 0;
if (r < 0)
return log_warning_errno(r, "Failed to receive notification message: %m");
return r;
Transfer *t;
HASHMAP_FOREACH(t, m->transfers)

View File

@@ -423,7 +423,7 @@ static int on_notify_socket(sd_event_source *s, int fd, unsigned event, void *us
if (r == -EAGAIN)
return 0;
if (r < 0)
return log_error_errno(r, "Failed to receive notification message: %m");
return r;
if (!pidref_equal(child, &pidref)) {
log_warning("Received notification message from unexpected process " PID_FMT " (expected " PID_FMT "), ignoring.",

View File

@@ -4626,7 +4626,7 @@ static int nspawn_dispatch_notify_fd(sd_event_source *source, int fd, uint32_t r
if (r == -EAGAIN)
return 0;
if (r < 0)
return log_error_errno(r, "Failed to receive notification message: %m");
return r;
if (sender_pid.pid != inner_child_pid) {
log_debug("Received notify message from process that is not the payload's PID 1. Ignoring.");

View File

@@ -4,11 +4,20 @@
#include "notify-recv.h"
#include "socket-util.h"
int notify_recv(int fd, char **ret_text, struct ucred *ret_ucred, PidRef *ret_pidref) {
int notify_recv(
int fd,
char **ret_text,
struct ucred *ret_ucred,
PidRef *ret_pidref) {
char buf[NOTIFY_BUFFER_MAX];
struct iovec iovec = {
.iov_base = buf,
.iov_len = sizeof(buf),
};
CMSG_BUFFER_TYPE(CMSG_SPACE(sizeof(struct ucred)) +
CMSG_SPACE(sizeof(int)) + /* SCM_PIDFD */
CMSG_SPACE(sizeof(int) * NOTIFY_FD_MAX)) control;
struct iovec iovec;
struct msghdr msghdr = {
.msg_iov = &iovec,
.msg_iovlen = 1,
@@ -16,38 +25,32 @@ int notify_recv(int fd, char **ret_text, struct ucred *ret_ucred, PidRef *ret_pi
.msg_controllen = sizeof(control),
};
ssize_t n;
int r;
assert(fd >= 0);
/* Receives a $NOTIFY_SOCKET message (aka sd_notify()). Does various validations. Returns -EAGAIN in
* case an invalid message is received (following the logic that an invalid message shall be ignored,
* and treated like no message at all). */
_cleanup_free_ char *buf = new(char, NOTIFY_BUFFER_MAX+1);
if (!buf)
return log_oom_debug();
iovec = (struct iovec) {
.iov_base = buf,
.iov_len = NOTIFY_BUFFER_MAX,
};
/* Receives a $NOTIFY_SOCKET message (aka sd_notify()). Does various validations.
*
* Returns -EAGAIN on recoverable errors (e.g. in case an invalid message is received, following
* the logic that an invalid message shall be ignored, and treated like no message at all). */
n = recvmsg_safe(fd, &msghdr, MSG_DONTWAIT|MSG_CMSG_CLOEXEC);
if (ERRNO_IS_NEG_TRANSIENT(n))
return -EAGAIN;
if (n == -ECHRNG) {
log_debug_errno(n, "Got message with truncated control data (unexpected fds sent?), ignoring.");
log_warning_errno(n, "Got message with truncated control data (unexpected fds sent?), ignoring.");
return -EAGAIN;
}
if (n == -EXFULL) {
log_debug_errno(n, "Got message with truncated payload data, ignoring.");
log_warning_errno(n, "Got message with truncated payload data, ignoring.");
return -EAGAIN;
}
if (n < 0)
return (int) n;
return log_error_errno(n, "Failed to receive notification message: %m");
const struct ucred *ucred = NULL;
_cleanup_close_ int pidfd = -EBADF;
struct cmsghdr *cmsg;
CMSG_FOREACH(cmsg, &msghdr) {
if (cmsg->cmsg_level != SOL_SOCKET)
@@ -63,28 +66,43 @@ int notify_recv(int fd, char **ret_text, struct ucred *ret_ucred, PidRef *ret_pi
case SCM_PIDFD:
assert(cmsg->cmsg_len == CMSG_LEN(sizeof(int)));
assert(pidfd < 0);
pidfd = *CMSG_TYPED_DATA(cmsg, int);
break;
case SCM_CREDENTIALS:
assert(cmsg->cmsg_len == CMSG_LEN(sizeof(struct ucred)));
assert(!ucred);
ucred = CMSG_TYPED_DATA(cmsg, struct ucred);
break;
}
}
if ((ret_ucred || ret_pidref) && (!ucred || ucred->pid <= 0))
return log_debug_errno(SYNTHETIC_ERRNO(EAGAIN), "Got notification datagram lacking valid credential information, ignoring.");
if ((ret_ucred || ret_pidref) && (!ucred || !pid_is_valid(ucred->pid)))
return log_warning_errno(SYNTHETIC_ERRNO(EAGAIN),
"Got notification datagram lacking valid credential information, ignoring.");
if (n == 0)
return log_debug_errno(SYNTHETIC_ERRNO(EAGAIN), "Got empty notification message, ignoring.");
if (memchr(buf, 0, n - 1))
return log_debug_errno(SYNTHETIC_ERRNO(EAGAIN), "Got notification message with embedded NUL, ignoring.");
/* As extra safety check, let's make sure the string we get doesn't contain embedded NUL bytes.
* We permit one trailing NUL byte in the message, but don't expect it. */
if (n > 1 && memchr(buf, 0, n - 1))
return log_warning_errno(SYNTHETIC_ERRNO(EAGAIN), "Got notification message with embedded NUL, ignoring.");
if (ret_text) {
char *s = memdup_suffix0(buf, n);
if (!s) {
log_oom_warning();
return -EAGAIN;
}
*ret_text = s;
}
if (ret_ucred)
*ret_ucred = *ucred;
if (ret_pidref) {
assert(ucred);
assert(ucred->pid > 0);
if (pidfd >= 0)
*ret_pidref = (PidRef) {
.pid = ucred->pid,
@@ -94,13 +112,5 @@ int notify_recv(int fd, char **ret_text, struct ucred *ret_ucred, PidRef *ret_pi
*ret_pidref = PIDREF_MAKE_FROM_PID(ucred->pid);
}
if (ret_text) {
buf[n] = 0;
*ret_text = TAKE_PTR(buf);
}
if (ret_ucred)
*ret_ucred = *ucred;
return 0;
}

View File

@@ -989,7 +989,7 @@ static int helper_on_notify(sd_event_source *s, int fd, uint32_t revents, void *
if (r == -EAGAIN)
return 0;
if (r < 0)
return log_warning_errno(r, "Failed to receive notification message: %m");
return r;
if (!pidref_equal(&ctx->pid, &sender_pid)) {
log_warning("Got notification datagram from unexpected peer, ignoring.");

View File

@@ -1657,7 +1657,7 @@ static int manager_on_notify(sd_event_source *s, int fd, uint32_t revents, void
if (r == -EAGAIN)
return 0;
if (r < 0)
return log_warning_errno(r, "Failed to receive notification message: %m");
return r;
Job *j;
HASHMAP_FOREACH(j, m->jobs) {