Patchwork [02,of,14] chg: add connectat as a utility function

login
register
mail settings
Submitter Jun Wu
Date April 10, 2016, 11:57 p.m.
Message ID <97fbb23cf84c03ce5932.1460332639@x1c>
Download mbox | patch
Permalink /patch/14500/
State Changes Requested
Delegated to: Yuya Nishihara
Headers show

Comments

Jun Wu - April 10, 2016, 11:57 p.m.
# HG changeset patch
# User Jun Wu <quark@fb.com>
# Date 1460322456 -3600
#      Sun Apr 10 22:07:36 2016 +0100
# Node ID 97fbb23cf84c03ce59325e9de94902f40c68c8c8
# Parent  cecc773636dcaeaf6d5130831f400a77fb065a96
chg: add connectat as a utility function

We are going to use connectat and some platforms (namely FreeBSD) has it own
native implementation. The signature is the same with FreeBSD's so we can
switch to its native implementation easily with C macro later.

Note: Sadly, connectat() is broken on FreeBSD now, as tested with FreeBSD
10.3-RELEASE (r297264) amd64. It will error out with ENOTCAPABLE.
Using ktrace and kdump, I got:

  2928 chg      CALL  connectat(0x3,0x4,0x7fffffffd7d8,0x6a)
  2928 chg      STRU  struct sockaddr { AF_LOCAL, server }
  2928 chg      NAMI  "server"
  2928 chg      CAP   operation requires <CAP_LOOKUP>,
                      process holds <CAP_READ,CAP_WRITE,...,CAP_LOOKUP,...>
  2928 chg      RET   connectat -1 errno 93 Capabilities insufficient

It looks like a bug in the capsicum framework. Let's just stick to our own
implementation for now.
timeless - April 11, 2016, 3:32 a.m.
Jun Wu wrote:
> We are going to use connectat and some platforms (namely FreeBSD) has it own


it => its
Yuya Nishihara - April 11, 2016, 12:39 p.m.
On Mon, 11 Apr 2016 00:57:19 +0100, Jun Wu wrote:
> # HG changeset patch
> # User Jun Wu <quark@fb.com>
> # Date 1460322456 -3600
> #      Sun Apr 10 22:07:36 2016 +0100
> # Node ID 97fbb23cf84c03ce59325e9de94902f40c68c8c8
> # Parent  cecc773636dcaeaf6d5130831f400a77fb065a96
> chg: add connectat as a utility function
> 
> We are going to use connectat and some platforms (namely FreeBSD) has it own
> native implementation. The signature is the same with FreeBSD's so we can
> switch to its native implementation easily with C macro later.

I'd rather give different function name than shadowing standard function
on some platforms.

> --- a/contrib/chg/util.c
> +++ b/contrib/chg/util.c
> @@ -8,6 +8,7 @@
>   */
>  
>  #include <errno.h>
> +#include <fcntl.h>
>  #include <signal.h>
>  #include <stdarg.h>
>  #include <stdio.h>
> @@ -178,3 +179,18 @@
>  		return -WTERMSIG(status);
>  	return 127;
>  }
> +
> +int connectat(int fd, int s, const struct sockaddr *addr, socklen_t addrlen)
> +{
> +	if (fd == AT_FDCWD)
> +		return connect(s, addr, addrlen);
> +	int cwdfd = open(".", O_DIRECTORY);
> +	if (cwdfd == -1)
> +		abortmsgerrno("cannot open cwd");
                ^^^^^^^^^^^^^
> +	fchdirx(fd);
        ^^^^^^^

Obviously this isn't compatible with the native implementation.

Patch

diff --git a/contrib/chg/util.c b/contrib/chg/util.c
--- a/contrib/chg/util.c
+++ b/contrib/chg/util.c
@@ -8,6 +8,7 @@ 
  */
 
 #include <errno.h>
+#include <fcntl.h>
 #include <signal.h>
 #include <stdarg.h>
 #include <stdio.h>
@@ -178,3 +179,18 @@ 
 		return -WTERMSIG(status);
 	return 127;
 }
+
+int connectat(int fd, int s, const struct sockaddr *addr, socklen_t addrlen)
+{
+	if (fd == AT_FDCWD)
+		return connect(s, addr, addrlen);
+	int cwdfd = open(".", O_DIRECTORY);
+	if (cwdfd == -1)
+		abortmsgerrno("cannot open cwd");
+	fchdirx(fd);
+	int r = connect(s, addr, addrlen);
+	fchdirx(cwdfd);
+	close(cwdfd);
+	return r;
+}
+
diff --git a/contrib/chg/util.h b/contrib/chg/util.h
--- a/contrib/chg/util.h
+++ b/contrib/chg/util.h
@@ -10,6 +10,8 @@ 
 #ifndef UTIL_H_
 #define UTIL_H_
 
+#include <sys/socket.h>
+
 #ifdef __GNUC__
 #define PRINTF_FORMAT_ __attribute__((format(printf, 1, 2)))
 #else
@@ -29,4 +31,6 @@ 
 
 int runshellcmd(const char *cmd, const char *envp[], const char *cwd);
 
+int connectat(int fd, int s, const struct sockaddr *name, socklen_t namelen);
+
 #endif  /* UTIL_H_ */