[systemd-devel] [PATCH 4/4] Always use recvmsg with MSG_CMSG_CLOEXEC

Lennart Poettering lennart at poettering.net
Tue Feb 10 12:26:00 PST 2015


On Tue, 10.02.15 12:06, Cristian Rodríguez (crrodriguez at opensuse.org) wrote:

Well, all but two of these recvmsg() invocations apply to network
sockets, which cannot receive fds, hence specifying the flag is kinda
pointless...

For the other two cases we should probably close all fds we
receive... I mean receiving them and not doing anything with them is
not any worse or better as doing so and having O_CLOEXEC set for
them...

That said, I think it probably makes sense to enforce the general rule
that recvmsg() must be invoked with MSG_CMSG_CLOEXEC, hence I applied
your patch and updated the CODING_STYLE to mention this.

> ---
>  src/libsystemd-network/sd-dhcp-client.c | 2 +-
>  src/libsystemd-network/sd-dhcp-server.c | 2 +-
>  src/libsystemd/sd-rtnl/rtnl-message.c   | 4 ++--
>  src/libudev/libudev-monitor.c           | 2 +-
>  src/machine/machine-dbus.c              | 2 +-
>  src/resolve/resolved-manager.c          | 2 +-
>  src/shutdownd/shutdownd.c               | 2 +-
>  src/timesync/timesyncd-manager.c        | 2 +-
>  src/udev/udev-ctrl.c                    | 2 +-
>  9 files changed, 10 insertions(+), 10 deletions(-)
> 
> diff --git a/src/libsystemd-network/sd-dhcp-client.c b/src/libsystemd-network/sd-dhcp-client.c
> index 5f90617..2f76e24 100644
> --- a/src/libsystemd-network/sd-dhcp-client.c
> +++ b/src/libsystemd-network/sd-dhcp-client.c
> @@ -1582,7 +1582,7 @@ static int client_receive_message_raw(sd_event_source *s, int fd,
>          iov.iov_base = packet;
>          iov.iov_len = buflen;
>  
> -        len = recvmsg(fd, &msg, 0);
> +        len = recvmsg(fd, &msg, MSG_CMSG_CLOEXEC);
>          if (len < 0) {
>                  log_dhcp_client(client, "could not receive message from raw "
>                                  "socket: %m");
> diff --git a/src/libsystemd-network/sd-dhcp-server.c b/src/libsystemd-network/sd-dhcp-server.c
> index 3f89f34..1cb782f 100644
> --- a/src/libsystemd-network/sd-dhcp-server.c
> +++ b/src/libsystemd-network/sd-dhcp-server.c
> @@ -897,7 +897,7 @@ static int server_receive_message(sd_event_source *s, int fd,
>          iov.iov_base = message;
>          iov.iov_len = buflen;
>  
> -        len = recvmsg(fd, &msg, 0);
> +        len = recvmsg(fd, &msg, MSG_CMSG_CLOEXEC);
>          if (len < buflen)
>                  return 0;
>          else if ((size_t)len < sizeof(DHCPMessage))
> diff --git a/src/libsystemd/sd-rtnl/rtnl-message.c b/src/libsystemd/sd-rtnl/rtnl-message.c
> index 276591f..e7e3799 100644
> --- a/src/libsystemd/sd-rtnl/rtnl-message.c
> +++ b/src/libsystemd/sd-rtnl/rtnl-message.c
> @@ -1431,7 +1431,7 @@ static int socket_recv_message(int fd, struct iovec *iov, uint32_t *_group, bool
>          assert(fd >= 0);
>          assert(iov);
>  
> -        r = recvmsg(fd, &msg, MSG_TRUNC | (peek ? MSG_PEEK : 0));
> +        r = recvmsg(fd, &msg, MSG_TRUNC | MSG_CMSG_CLOEXEC | (peek ? MSG_PEEK : 0));
>          if (r < 0) {
>                  /* no data */
>                  if (errno == ENOBUFS)
> @@ -1467,7 +1467,7 @@ static int socket_recv_message(int fd, struct iovec *iov, uint32_t *_group, bool
>                  /* not from the kernel, ignore */
>                  if (peek) {
>                          /* drop the message */
> -                        r = recvmsg(fd, &msg, 0);
> +                        r = recvmsg(fd, &msg, MSG_CMSG_CLOEXEC);
>                          if (r < 0)
>                                  return (errno == EAGAIN || errno == EINTR) ? 0 : -errno;
>                  }
> diff --git a/src/libudev/libudev-monitor.c b/src/libudev/libudev-monitor.c
> index 08ddde8..82ce7f6 100644
> --- a/src/libudev/libudev-monitor.c
> +++ b/src/libudev/libudev-monitor.c
> @@ -600,7 +600,7 @@ retry:
>          smsg.msg_name = &snl;
>          smsg.msg_namelen = sizeof(snl);
>  
> -        buflen = recvmsg(udev_monitor->sock, &smsg, 0);
> +        buflen = recvmsg(udev_monitor->sock, &smsg, MSG_CMSG_CLOEXEC);
>          if (buflen < 0) {
>                  if (errno != EINTR)
>                          log_debug("unable to receive message");
> diff --git a/src/machine/machine-dbus.c b/src/machine/machine-dbus.c
> index b46f0a8..da8e6c0 100644
> --- a/src/machine/machine-dbus.c
> +++ b/src/machine/machine-dbus.c
> @@ -255,7 +255,7 @@ int bus_machine_method_get_addresses(sd_bus *bus, sd_bus_message *message, void
>                  iov[0] = (struct iovec) { .iov_base = &family, .iov_len = sizeof(family) };
>                  iov[1] = (struct iovec) { .iov_base = &in_addr, .iov_len = sizeof(in_addr) };
>  
> -                n = recvmsg(pair[0], &mh, 0);
> +                n = recvmsg(pair[0], &mh, MSG_CMSG_CLOEXEC);
>                  if (n < 0)
>                          return sd_bus_error_set_errno(error, -errno);
>                  if ((size_t) n < sizeof(family))
> diff --git a/src/resolve/resolved-manager.c b/src/resolve/resolved-manager.c
> index 890cc04..d5c1bf0 100644
> --- a/src/resolve/resolved-manager.c
> +++ b/src/resolve/resolved-manager.c
> @@ -892,7 +892,7 @@ int manager_recv(Manager *m, int fd, DnsProtocol protocol, DnsPacket **ret) {
>          mh.msg_control = &control;
>          mh.msg_controllen = sizeof(control);
>  
> -        l = recvmsg(fd, &mh, 0);
> +        l = recvmsg(fd, &mh, MSG_CMSG_CLOEXEC);
>          if (l < 0) {
>                  if (errno == EAGAIN || errno == EINTR)
>                          return 0;
> diff --git a/src/shutdownd/shutdownd.c b/src/shutdownd/shutdownd.c
> index 826efbf..6eb522b 100644
> --- a/src/shutdownd/shutdownd.c
> +++ b/src/shutdownd/shutdownd.c
> @@ -69,7 +69,7 @@ static int read_packet(int fd, union shutdown_buffer *_b) {
>          assert(fd >= 0);
>          assert(_b);
>  
> -        n = recvmsg(fd, &msghdr, MSG_DONTWAIT);
> +        n = recvmsg(fd, &msghdr, MSG_DONTWAIT|MSG_CMSG_CLOEXEC);
>          if (n <= 0) {
>                  if (n == 0) {
>                          log_error("Short read");
> diff --git a/src/timesync/timesyncd-manager.c b/src/timesync/timesyncd-manager.c
> index 223671c..edddc2f 100644
> --- a/src/timesync/timesyncd-manager.c
> +++ b/src/timesync/timesyncd-manager.c
> @@ -516,7 +516,7 @@ static int manager_receive_response(sd_event_source *source, int fd, uint32_t re
>                  return manager_connect(m);
>          }
>  
> -        len = recvmsg(fd, &msghdr, MSG_DONTWAIT);
> +        len = recvmsg(fd, &msghdr, MSG_DONTWAIT|MSG_CMSG_CLOEXEC);
>          if (len < 0) {
>                  if (errno == EAGAIN)
>                          return 0;
> diff --git a/src/udev/udev-ctrl.c b/src/udev/udev-ctrl.c
> index 538b342..47f0651 100644
> --- a/src/udev/udev-ctrl.c
> +++ b/src/udev/udev-ctrl.c
> @@ -372,7 +372,7 @@ struct udev_ctrl_msg *udev_ctrl_receive_msg(struct udev_ctrl_connection *conn) {
>          iov.iov_base = &uctrl_msg->ctrl_msg_wire;
>          iov.iov_len = sizeof(struct udev_ctrl_msg_wire);
>  
> -        size = recvmsg(conn->sock, &smsg, 0);
> +        size = recvmsg(conn->sock, &smsg, MSG_CMSG_CLOEXEC);
>          if (size <  0) {
>                  log_error_errno(errno, "unable to receive ctrl message: %m");
>                  goto err;
> -- 
> 2.2.2
> 
> _______________________________________________
> systemd-devel mailing list
> systemd-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/systemd-devel


Lennart

-- 
Lennart Poettering, Red Hat


More information about the systemd-devel mailing list