From 2d651ad2d0b03bf9cc59f17c711159a4d4ae6c46 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Th=C3=A9ophane=20Hufschmitt?= Date: Wed, 15 Jun 2022 18:07:13 +0200 Subject: [PATCH] Always throw the right exception in `connect` When `nix::connect` is called with a socket path that's too long, it forks a process that will `chdir` to the directory of the socket path and call `::connect` with the relative path (which is hopefully short-enough). That works fairly well, except that an exception raised in this subprocess won't be forwarded to the parent process. Instead the logic will just notice that the subprocess exited with a non-zero error code, and throw a generic `Error`. In particular, any failure in the `::connect` call should throw a `SysError` with the correct error code, but that's not the case. Some places try to catch this `SysError` and look at its error code (to potentially restart for example). But this doesn't work since the actual error that gets thrown isn't a `SysError`. Fix that by forwarding the `errno` in case something gets wrong (by setting the subprocess exit code to it), and throwing a `SysError` with the right error code in the parent process. --- src/libutil/util.cc | 35 ++++++++++++++++++++++++----------- 1 file changed, 24 insertions(+), 11 deletions(-) diff --git a/src/libutil/util.cc b/src/libutil/util.cc index 4da1379cc..619285e6b 100644 --- a/src/libutil/util.cc +++ b/src/libutil/util.cc @@ -1817,19 +1817,32 @@ void connect(int fd, const std::string & path) if (path.size() + 1 >= sizeof(addr.sun_path)) { Pid pid = startProcess([&]() { - Path dir = dirOf(path); - if (chdir(dir.c_str()) == -1) - throw SysError("chdir to '%s' failed", dir); - std::string base(baseNameOf(path)); - if (base.size() + 1 >= sizeof(addr.sun_path)) - throw Error("socket path '%s' is too long", base); - memcpy(addr.sun_path, base.c_str(), base.size() + 1); - if (connect(fd, (struct sockaddr *) &addr, sizeof(addr)) == -1) - throw SysError("cannot connect to socket at '%s'", path); - _exit(0); + try { + Path dir = dirOf(path); + if (chdir(dir.c_str()) == -1) + throw SysError("chdir to '%s' failed", dir); + std::string base(baseNameOf(path)); + if (base.size() + 1 >= sizeof(addr.sun_path)) + throw Error("socket path '%s' is too long", base); + memcpy(addr.sun_path, base.c_str(), base.size() + 1); + if (connect(fd, (struct sockaddr *) &addr, sizeof(addr)) == -1) + throw SysError("cannot connect to socket at '%s'", path); + _exit(0); + } catch (SysError & e) { + // If this raised a `SysError`, we want to rethrow it in the + // parent process. + // For that, we need to transmit the associated error code, + // which we do by setting it as the return code for the process. + _exit(e.errNo); + } catch (std::exception &) { + _exit(-1); + } }); int status = pid.wait(); - if (status != 0) + if (status > 0) { + errno = status; + throw SysError("cannot connect to socket at '%s'", path); + } else if (status < 0) throw Error("cannot connect to socket at '%s'", path); } else { memcpy(addr.sun_path, path.c_str(), path.size() + 1);