From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by passt.top (Postfix) with ESMTP id CD4765A005E for ; Thu, 9 Feb 2023 18:45:59 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1675964758; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=TZeCi6/RSS7Py40mBAo5dkk2Y++M7FTLdpbFKVQdkxU=; b=RFkbrVyYNFgTXeI7l0RuSkwsmr/snL3cjwhnQahjE5LX5QiZNSRVKsVnpLzpEHNTX21KCq y/n3scmd71+AFjNi8SHHkQfy09JjNX2jYomiSWrBv5zxRN4l4Vy8tK2NinAAwe3NIqavS9 uH+QpygSFHees9IiKtEuZfbix6/iTz4= Received: from mimecast-mx02.redhat.com (mx3-rdu2.redhat.com [66.187.233.73]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id us-mta-528-CQ8S4HMrMA213jOgmU-gdA-1; Thu, 09 Feb 2023 12:45:55 -0500 X-MC-Unique: CQ8S4HMrMA213jOgmU-gdA-1 Received: from smtp.corp.redhat.com (int-mx09.intmail.prod.int.rdu2.redhat.com [10.11.54.9]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx02.redhat.com (Postfix) with ESMTPS id F03D93C01DF0; Thu, 9 Feb 2023 17:45:54 +0000 (UTC) Received: from maya.cloud.tilaa.com (ovpn-208-4.brq.redhat.com [10.40.208.4]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 9219E492C3F; Thu, 9 Feb 2023 17:45:54 +0000 (UTC) Date: Thu, 9 Feb 2023 18:45:52 +0100 From: Stefano Brivio To: Laine Stump Subject: Re: [PATCH v2 9/9] convert all remaining err() followed by exit() to errexit() Message-ID: <20230209184552.4dc5ab98@elisabeth> In-Reply-To: <20230208174838.1680517-10-laine@redhat.com> References: <20230208174838.1680517-1-laine@redhat.com> <20230208174838.1680517-10-laine@redhat.com> Organization: Red Hat MIME-Version: 1.0 X-Scanned-By: MIMEDefang 3.1 on 10.11.54.9 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Message-ID-Hash: 5NPZNRF5BIV5R7EH3TS2VZXDF7GYLRDL X-Message-ID-Hash: 5NPZNRF5BIV5R7EH3TS2VZXDF7GYLRDL X-MailFrom: sbrivio@redhat.com X-Mailman-Rule-Misses: dmarc-mitigation; no-senders; approved; emergency; loop; banned-address; member-moderation; nonmember-moderation; administrivia; implicit-dest; max-recipients; max-size; news-moderation; no-subject; digests; suspicious-header CC: passt-dev@passt.top, laine@laine.org X-Mailman-Version: 3.3.3 Precedence: list List-Id: Development discussion and patches for passt Archived-At: Archived-At: List-Archive: List-Archive: List-Help: List-Owner: List-Post: List-Subscribe: List-Unsubscribe: On Wed, 8 Feb 2023 12:48:38 -0500 Laine Stump wrote: > This actually leaves us with 0 uses of err(), but someone could want > to use it in the future, so may as well leave it around. > > Signed-off-by: Laine Stump > --- > isolation.c | 78 +++++++++++++++++++---------------------------------- > log.c | 6 ++--- > netlink.c | 3 +-- > passt.c | 12 +++------ > pasta.c | 21 ++++++--------- > tap.c | 30 +++++++-------------- > 6 files changed, 53 insertions(+), 97 deletions(-) > > diff --git a/isolation.c b/isolation.c > index 4e6637d..0f709c6 100644 > --- a/isolation.c > +++ b/isolation.c > @@ -103,10 +103,8 @@ static void drop_caps_ep_except(uint64_t keep) > struct __user_cap_data_struct data[CAP_WORDS]; > int i; > > - if (syscall(SYS_capget, &hdr, data)) { > - err("Couldn't get current capabilities: %s", strerror(errno)); > - exit(EXIT_FAILURE); > - } > + if (syscall(SYS_capget, &hdr, data)) > + errexit("Couldn't get current capabilities: %s", strerror(errno)); Split before strerror(). > > for (i = 0; i < CAP_WORDS; i++) { > uint32_t mask = keep >> (32 * i); > @@ -115,10 +113,8 @@ static void drop_caps_ep_except(uint64_t keep) > data[i].permitted &= mask; > } > > - if (syscall(SYS_capset, &hdr, data)) { > - err("Couldn't drop capabilities: %s", strerror(errno)); > - exit(EXIT_FAILURE); > - } > + if (syscall(SYS_capset, &hdr, data)) > + errexit("Couldn't drop capabilities: %s", strerror(errno)); > } > > /** > @@ -154,26 +150,20 @@ static void clamp_caps(void) > * normal operation, so carry on without it. > */ > if (prctl(PR_CAPBSET_DROP, i, 0, 0, 0) && > - errno != EINVAL && errno != EPERM) { > - err("Couldn't drop cap %i from bounding set: %s", > - i, strerror(errno)); > - exit(EXIT_FAILURE); > - } > + errno != EINVAL && errno != EPERM) > + errexit("Couldn't drop cap %i from bounding set: %s", > + i, strerror(errno)); > } > > - if (syscall(SYS_capget, &hdr, data)) { > - err("Couldn't get current capabilities: %s", strerror(errno)); > - exit(EXIT_FAILURE); > - } > + if (syscall(SYS_capget, &hdr, data)) > + errexit("Couldn't get current capabilities: %s", strerror(errno)); Same here. > > for (i = 0; i < CAP_WORDS; i++) > data[i].inheritable = 0; > > - if (syscall(SYS_capset, &hdr, data)) { > - err("Couldn't drop inheritable capabilities: %s", > - strerror(errno)); > - exit(EXIT_FAILURE); > - } > + if (syscall(SYS_capset, &hdr, data)) > + errexit("Couldn't drop inheritable capabilities: %s", > + strerror(errno)); > } > > /** > @@ -229,46 +219,34 @@ void isolate_user(uid_t uid, gid_t gid, bool use_userns, const char *userns, > /* First set our UID & GID in the original namespace */ > if (setgroups(0, NULL)) { > /* If we don't have CAP_SETGID, this will EPERM */ > - if (errno != EPERM) { > - err("Can't drop supplementary groups: %s", > - strerror(errno)); > - exit(EXIT_FAILURE); > - } > + if (errno != EPERM) > + errexit("Can't drop supplementary groups: %s", > + strerror(errno)); > } > > - if (setgid(gid) != 0) { > - err("Can't set GID to %u: %s", gid, strerror(errno)); > - exit(EXIT_FAILURE); > - } > + if (setgid(gid) != 0) > + errexit("Can't set GID to %u: %s", gid, strerror(errno)); > > - if (setuid(uid) != 0) { > - err("Can't set UID to %u: %s", uid, strerror(errno)); > - exit(EXIT_FAILURE); > - } > + if (setuid(uid) != 0) > + errexit("Can't set UID to %u: %s", uid, strerror(errno)); > > if (*userns) { /* If given a userns, join it */ > int ufd; > > ufd = open(userns, O_RDONLY | O_CLOEXEC); > - if (ufd < 0) { > - err("Couldn't open user namespace %s: %s", > - userns, strerror(errno)); > - exit(EXIT_FAILURE); > - } > - > - if (setns(ufd, CLONE_NEWUSER) != 0) { > - err("Couldn't enter user namespace %s: %s", > - userns, strerror(errno)); > - exit(EXIT_FAILURE); > - } > + if (ufd < 0) > + errexit("Couldn't open user namespace %s: %s", > + userns, strerror(errno)); > + > + if (setns(ufd, CLONE_NEWUSER) != 0) > + errexit("Couldn't enter user namespace %s: %s", > + userns, strerror(errno)); > > close(ufd); > > } else if (use_userns) { /* Create and join a new userns */ > - if (unshare(CLONE_NEWUSER) != 0) { > - err("Couldn't create user namespace: %s", strerror(errno)); > - exit(EXIT_FAILURE); > - } > + if (unshare(CLONE_NEWUSER) != 0) > + errexit("Couldn't create user namespace: %s", strerror(errno)); And here. > } > > /* Joining a new userns gives us full capabilities; drop the > diff --git a/log.c b/log.c > index 4956914..983c82f 100644 > --- a/log.c > +++ b/log.c > @@ -204,10 +204,8 @@ void logfile_init(const char *name, const char *path, size_t size) > > log_file = open(path, O_CREAT | O_TRUNC | O_APPEND | O_RDWR | O_CLOEXEC, > S_IRUSR | S_IWUSR); > - if (log_file == -1) { > - err("Couldn't open log file %s: %s", path, strerror(errno)); > - exit(EXIT_FAILURE); > - } > + if (log_file == -1) > + errexit("Couldn't open log file %s: %s", path, strerror(errno)); > > log_size = size ? size : LOGFILE_SIZE_DEFAULT; > > diff --git a/netlink.c b/netlink.c > index 0850cbe..a6c6e1e 100644 > --- a/netlink.c > +++ b/netlink.c > @@ -90,8 +90,7 @@ void nl_sock_init(const struct ctx *c, bool ns) > return; > > fail: > - err("Failed to get netlink socket"); > - exit(EXIT_FAILURE); > + errexit("Failed to get netlink socket"); > } > > /** > diff --git a/passt.c b/passt.c > index cf010e8..443d51a 100644 > --- a/passt.c > +++ b/passt.c > @@ -202,10 +202,8 @@ int main(int argc, char **argv) > name = basename(argv0); > if (strstr(name, "pasta")) { > sa.sa_handler = pasta_child_handler; > - if (sigaction(SIGCHLD, &sa, NULL) || signal(SIGPIPE, SIG_IGN)) { > - err("Couldn't install signal handlers"); > - exit(EXIT_FAILURE); > - } > + if (sigaction(SIGCHLD, &sa, NULL) || signal(SIGPIPE, SIG_IGN)) > + errexit("Couldn't install signal handlers"); > > c.mode = MODE_PASTA; > log_name = "pasta"; > @@ -291,10 +289,8 @@ int main(int argc, char **argv) > } > } > > - if (isolate_prefork(&c)) { > - err("Failed to sandbox process, exiting\n"); > - exit(EXIT_FAILURE); > - } > + if (isolate_prefork(&c)) > + errexit("Failed to sandbox process, exiting\n"); > > if (!c.foreground) { > __daemon(pidfile_fd, devnull_fd); > diff --git a/pasta.c b/pasta.c > index 528f02a..b1463c9 100644 > --- a/pasta.c > +++ b/pasta.c > @@ -123,19 +123,15 @@ void pasta_open_ns(struct ctx *c, const char *netns) > int nfd = -1; > > nfd = open(netns, O_RDONLY | O_CLOEXEC); > - if (nfd < 0) { > - err("Couldn't open network namespace %s", netns); > - exit(EXIT_FAILURE); > - } > + if (nfd < 0) > + errexit("Couldn't open network namespace %s", netns); > > c->pasta_netns_fd = nfd; > > NS_CALL(ns_check, c); > > - if (c->pasta_netns_fd < 0) { > - err("Couldn't switch to pasta namespaces"); > - exit(EXIT_FAILURE); > - } > + if (c->pasta_netns_fd < 0) > + errexit("Couldn't switch to pasta namespaces"); > > if (!c->no_netns_quit) { > char buf[PATH_MAX] = { 0 }; > @@ -217,11 +213,10 @@ void pasta_start_ns(struct ctx *c, uid_t uid, gid_t gid, > arg.exe = "/bin/sh"; > > if ((size_t)snprintf(sh_arg0, sizeof(sh_arg0), > - "-%s", arg.exe) >= sizeof(sh_arg0)) { > - err("$SHELL is too long (%u bytes)", > - strlen(arg.exe)); > - exit(EXIT_FAILURE); > - } > + "-%s", arg.exe) >= sizeof(sh_arg0)) > + errexit("$SHELL is too long (%u bytes)", > + strlen(arg.exe)); > + > sh_argv[0] = sh_arg0; > arg.argv = sh_argv; > } > diff --git a/tap.c b/tap.c > index 2e603ed..edb3184 100644 > --- a/tap.c > +++ b/tap.c > @@ -886,10 +886,8 @@ static void tap_sock_unix_init(struct ctx *c) > }; > int i; > > - if (fd < 0) { > - err("UNIX socket: %s", strerror(errno)); > - exit(EXIT_FAILURE); > - } > + if (fd < 0) > + errexit("UNIX socket: %s", strerror(errno)); > > /* In passt mode, we don't know the guest's MAC until it sends > * us packets. Use the broadcast address so our first packets > @@ -907,18 +905,14 @@ static void tap_sock_unix_init(struct ctx *c) > snprintf(path, UNIX_PATH_MAX - 1, UNIX_SOCK_PATH, i); > > ex = socket(AF_UNIX, SOCK_STREAM | SOCK_NONBLOCK, 0); > - if (ex < 0) { > - err("UNIX domain socket check: %s", strerror(errno)); > - exit(EXIT_FAILURE); > - } > + if (ex < 0) > + errexit("UNIX domain socket check: %s", strerror(errno)); And here. > > ret = connect(ex, (const struct sockaddr *)&addr, sizeof(addr)); > if (!ret || (errno != ENOENT && errno != ECONNREFUSED && > errno != EACCES)) { > - if (*c->sock_path) { > - err("Socket path %s already in use", path); > - exit(EXIT_FAILURE); > - } > + if (*c->sock_path) > + errexit("Socket path %s already in use", path); > > close(ex); > continue; > @@ -931,10 +925,8 @@ static void tap_sock_unix_init(struct ctx *c) > break; > } > > - if (i == UNIX_SOCK_MAX) { > - err("UNIX socket bind: %s", strerror(errno)); > - exit(EXIT_FAILURE); > - } > + if (i == UNIX_SOCK_MAX) > + errexit("UNIX socket bind: %s", strerror(errno)); > > info("UNIX domain socket bound at %s\n", addr.sun_path); > > @@ -1037,10 +1029,8 @@ static void tap_sock_tun_init(struct ctx *c) > struct epoll_event ev = { 0 }; > > NS_CALL(tap_ns_tun, c); > - if (tun_ns_fd == -1) { > - err("Failed to open tun socket in namespace"); > - exit(EXIT_FAILURE); > - } > + if (tun_ns_fd == -1) > + errexit("Failed to open tun socket in namespace"); > > pasta_ns_conf(c); > -- Stefano