On 21/01/17(Sat) 20:40, Philip Guenther wrote:
>
> This is your periodic reminder that it's pointless for userspace to set
> the {sun,sin,sin6,etc}_len field in sockaddrs being passed to the POSIX
> APIs. An application can use it internally for its own purposes, but the
> kernel always overwrites the value when passed in.
I'd say explicit is better than implicit. Is there any bug in the code
below?
I believe it is simpler to always set the length, it makes our life easier
when review code between kernel and userland. Or should we have two
different practices because history screwed up?
>
> There are more of these that can be cleaned up, but here's the chunk I've
> had sitting in my tree.
>
> oks for any of these?
>
>
> Philip
>
>
> Index: usr.bin/netstat/inet6.c
> ===================================================================
> RCS file: /data/src/openbsd/src/usr.bin/netstat/inet6.c,v
> retrieving revision 1.51
> diff -u -p -r1.51 inet6.c
> --- usr.bin/netstat/inet6.c 22 Dec 2016 11:04:44 -0000 1.51
> +++ usr.bin/netstat/inet6.c 23 Dec 2016 22:55:54 -0000
> @@ -983,7 +983,6 @@ inet6name(struct in6_addr *in6p)
> strlcpy(line, cp, sizeof(line));
> else {
> memset(&sin6, 0, sizeof(sin6));
> - sin6.sin6_len = sizeof(sin6);
> sin6.sin6_family = AF_INET6;
> sin6.sin6_addr = *in6p;
> #ifdef __KAME__
> @@ -996,7 +995,7 @@ inet6name(struct in6_addr *in6p)
> sin6.sin6_addr.s6_addr[3] = 0;
> }
> #endif
> - if (getnameinfo((struct sockaddr *)&sin6, sin6.sin6_len,
> + if (getnameinfo((struct sockaddr *)&sin6, sizeof(sin6),
> hbuf, sizeof(hbuf), NULL, 0, niflag) != 0)
> strlcpy(hbuf, "?", sizeof hbuf);
> strlcpy(line, hbuf, sizeof(line));
> Index: usr.bin/netstat/show.c
> ===================================================================
> RCS file: /data/src/openbsd/src/usr.bin/netstat/show.c,v
> retrieving revision 1.52
> diff -u -p -r1.52 show.c
> --- usr.bin/netstat/show.c 3 Sep 2016 14:23:14 -0000 1.52
> +++ usr.bin/netstat/show.c 3 Sep 2016 14:23:32 -0000
> @@ -449,7 +449,6 @@ routename(struct sockaddr *sa)
>
> memset(&sin6, 0, sizeof(sin6));
> memcpy(&sin6, sa, sa->sa_len);
> - sin6.sin6_len = sizeof(struct sockaddr_in6);
> sin6.sin6_family = AF_INET6;
> if (sa->sa_len == sizeof(struct sockaddr_in6) &&
> (IN6_IS_ADDR_LINKLOCAL(&sin6.sin6_addr) ||
> @@ -520,7 +519,7 @@ routename6(struct sockaddr_in6 *sin6)
> else
> niflags |= NI_NOFQDN;
>
> - if (getnameinfo((struct sockaddr *)sin6, sin6->sin6_len,
> + if (getnameinfo((struct sockaddr *)sin6, sizeof(*sin6),
> line, sizeof(line), NULL, 0, niflags) != 0)
> strncpy(line, "invalid", sizeof(line));
>
> @@ -640,7 +639,7 @@ netname6(struct sockaddr_in6 *sa6, struc
>
> if (nflag)
> flag |= NI_NUMERICHOST;
> - error = getnameinfo((struct sockaddr *)&sin6, sin6.sin6_len,
> + error = getnameinfo((struct sockaddr *)&sin6, sizeof(sin6),
> hbuf, sizeof(hbuf), NULL, 0, flag);
> if (error)
> snprintf(hbuf, sizeof(hbuf), "invalid");
> Index: usr.bin/rusers/rusers.c
> ===================================================================
> RCS file: /data/src/openbsd/src/usr.bin/rusers/rusers.c,v
> retrieving revision 1.39
> diff -u -p -r1.39 rusers.c
> --- usr.bin/rusers/rusers.c 5 Aug 2016 10:34:18 -0000 1.39
> +++ usr.bin/rusers/rusers.c 6 Aug 2016 18:41:08 -0000
> @@ -418,7 +418,7 @@ retry:
> msgp->acpted_rply.ar_results.where = (caddr_t)resp;
> msgp->acpted_rply.ar_results.proc = xdr_rmtcallres;
>
> - fromlen = sizeof(struct sockaddr);
> + fromlen = sizeof(raddr);
> inlen = recvfrom(sock, inbuf, sizeof(inbuf), 0,
> (struct sockaddr *)&raddr, &fromlen);
> if (inlen < 0) {
> @@ -534,7 +534,6 @@ allhosts(void)
> outlen[1] = xdr_getpos(&xdr);
> xdr_destroy(&xdr);
>
> - baddr.sin_len = sizeof(struct sockaddr_in);
> baddr.sin_family = AF_INET;
> baddr.sin_port = htons(PMAPPORT);
> baddr.sin_addr.s_addr = htonl(INADDR_ANY);
> @@ -572,12 +571,12 @@ allhosts(void)
> if (i & 1) {
> if (sendto(sock[0], buf[0], outlen[0], 0,
> (struct sockaddr *)&baddr,
> - sizeof(struct sockaddr)) != outlen[0])
> + sizeof(baddr)) != outlen[0])
> err(1, "can't send broadcast packet");
> } else {
> if (sendto(sock[1], buf[1], outlen[1], 0,
> (struct sockaddr *)&baddr,
> - sizeof(struct sockaddr)) != outlen[1])
> + sizeof(baddr)) != outlen[1])
> err(1, "can't send broadcast packet");
> }
> }
> Index: usr.bin/showmount/showmount.c
> ===================================================================
> RCS file: /data/src/openbsd/src/usr.bin/showmount/showmount.c,v
> retrieving revision 1.20
> diff -u -p -r1.20 showmount.c
> --- usr.bin/showmount/showmount.c 16 Mar 2016 15:41:11 -0000 1.20
> +++ usr.bin/showmount/showmount.c 20 Mar 2016 02:36:18 -0000
> @@ -150,7 +150,6 @@ main(int argc, char *argv[])
> exit(1);
> }
> bzero(&clnt_sin, sizeof clnt_sin);
> - clnt_sin.sin_len = sizeof clnt_sin;
> clnt_sin.sin_family = AF_INET;
> bcopy(hp->h_addr, (char *)&clnt_sin.sin_addr, hp->h_length);
> clnt_sock = RPC_ANYSOCK;
> Index: usr.bin/ssh/mux.c
> ===================================================================
> RCS file: /data/src/openbsd/src/usr.bin/ssh/mux.c,v
> retrieving revision 1.63
> diff -u -p -r1.63 mux.c
> --- usr.bin/ssh/mux.c 19 Oct 2016 23:21:56 -0000 1.63
> +++ usr.bin/ssh/mux.c 21 Oct 2016 05:12:17 -0000
> @@ -2163,8 +2163,6 @@ muxclient(const char *path)
>
> memset(&addr, '\0', sizeof(addr));
> addr.sun_family = AF_UNIX;
> - addr.sun_len = offsetof(struct sockaddr_un, sun_path) +
> - strlen(path) + 1;
>
> if (strlcpy(addr.sun_path, path,
> sizeof(addr.sun_path)) >= sizeof(addr.sun_path))
> @@ -2174,7 +2172,7 @@ muxclient(const char *path)
> if ((sock = socket(PF_UNIX, SOCK_STREAM, 0)) < 0)
> fatal("%s socket(): %s", __func__, strerror(errno));
>
> - if (connect(sock, (struct sockaddr *)&addr, addr.sun_len) == -1) {
> + if (connect(sock, (struct sockaddr *)&addr, sizeof(addr)) == -1) {
> switch (muxclient_command) {
> case SSHMUX_COMMAND_OPEN:
> case SSHMUX_COMMAND_STDIO_FWD:
> Index: usr.bin/systat/inetname.c
> ===================================================================
> RCS file: /data/src/openbsd/src/usr.bin/systat/inetname.c,v
> retrieving revision 1.2
> diff -u -p -r1.2 inetname.c
> --- usr.bin/systat/inetname.c 16 Jan 2015 00:03:37 -0000 1.2
> +++ usr.bin/systat/inetname.c 11 Oct 2015 20:42:14 -0000
> @@ -52,9 +52,8 @@ inet6name(struct in6_addr *in6)
> return "*";
> memset(&sin6, 0, sizeof(sin6));
> sin6.sin6_family = AF_INET6;
> - sin6.sin6_len = sizeof(struct sockaddr_in6);
> sin6.sin6_addr = *in6;
> - if (getnameinfo((struct sockaddr *)&sin6, sin6.sin6_len,
> + if (getnameinfo((struct sockaddr *)&sin6, sizeof(struct
> sockaddr_in6),
> line, sizeof(line), NULL, 0, flags) == 0)
> return line;
> return "?";
> @@ -73,10 +72,9 @@ inetname(struct in_addr in)
>
> memset(&si, 0, sizeof(si));
> si.sin_family = AF_INET;
> - si.sin_len = sizeof(struct sockaddr_in);
> si.sin_addr = in;
>
> - e = getnameinfo((struct sockaddr *)&si, si.sin_len,
> + e = getnameinfo((struct sockaddr *)&si, sizeof(struct sockaddr_in),
> line, sizeof(line), NULL, 0, flags);
>
> if (e == 0)
> Index: usr.bin/tftp/main.c
> ===================================================================
> RCS file: /data/src/openbsd/src/usr.bin/tftp/main.c,v
> retrieving revision 1.40
> diff -u -p -r1.40 main.c
> --- usr.bin/tftp/main.c 16 Mar 2016 15:41:11 -0000 1.40
> +++ usr.bin/tftp/main.c 20 Mar 2016 02:36:19 -0000
> @@ -232,8 +232,7 @@ setpeer(char *host, char *port)
>
> memset(&ss, 0, sizeof(ss));
> ss.ss_family = res->ai_family;
> - ss.ss_len = res->ai_addrlen;
> - if (bind(f, (struct sockaddr *)&ss, ss.ss_len) < 0) {
> + if (bind(f, (struct sockaddr *)&ss, res->ai_addrlen) < 0) {
> cause = "bind";
> close(f);
> f = -1;
> Index: usr.sbin/httpd/server_fcgi.c
> ===================================================================
> RCS file: /data/src/openbsd/src/usr.sbin/httpd/server_fcgi.c,v
> retrieving revision 1.73
> diff -u -p -r1.73 server_fcgi.c
> --- usr.sbin/httpd/server_fcgi.c 7 Oct 2016 07:37:29 -0000 1.73
> +++ usr.sbin/httpd/server_fcgi.c 8 Oct 2016 05:16:40 -0000
> @@ -120,7 +120,6 @@ server_fcgi(struct httpd *env, struct cl
> goto fail;
> } else {
> struct sockaddr_un sun;
> - size_t len;
>
> if ((fd = socket(AF_UNIX,
> SOCK_STREAM | SOCK_NONBLOCK, 0)) == -1)
> @@ -128,13 +127,11 @@ server_fcgi(struct httpd *env, struct cl
>
> memset(&sun, 0, sizeof(sun));
> sun.sun_family = AF_UNIX;
> - len = strlcpy(sun.sun_path,
> - srv_conf->socket, sizeof(sun.sun_path));
> - if (len >= sizeof(sun.sun_path)) {
> - errstr = "socket path too long";
> + if (strlcpy(sun.sun_path, srv_conf->socket,
> + sizeof(sun.sun_path)) >= sizeof(sun.sun_path)) {
> + errstr = "socket path to long";
> goto fail;
> }
> - sun.sun_len = len;
>
> if (connect(fd, (struct sockaddr *)&sun, sizeof(sun)) == -1)
> goto fail;
> Index: usr.sbin/mrouted/rsrr.c
> ===================================================================
> RCS file: /data/src/openbsd/src/usr.sbin/mrouted/rsrr.c,v
> retrieving revision 1.14
> diff -u -p -r1.14 rsrr.c
> --- usr.sbin/mrouted/rsrr.c 21 Aug 2015 02:07:32 -0000 1.14
> +++ usr.sbin/mrouted/rsrr.c 23 Aug 2015 00:24:37 -0000
> @@ -83,7 +83,6 @@ static void rsrr_cache(struct gtable *gt
> void
> rsrr_init(void)
> {
> - int servlen;
> struct sockaddr_un serv_addr;
>
> if ((rsrr_socket = socket(AF_UNIX, SOCK_DGRAM, 0)) < 0)
> @@ -93,11 +92,8 @@ rsrr_init(void)
> bzero((char *) &serv_addr, sizeof(serv_addr));
> serv_addr.sun_family = AF_UNIX;
> strlcpy(serv_addr.sun_path, RSRR_SERV_PATH, sizeof serv_addr.sun_path);
> - servlen = offsetof(struct sockaddr_un, sun_path) +
> - strlen(serv_addr.sun_path);
> - serv_addr.sun_len = servlen;
>
> - if (bind(rsrr_socket, (struct sockaddr *) &serv_addr, servlen) < 0)
> + if (bind(rsrr_socket, (struct sockaddr *)&serv_addr, sizeof serv_addr) <
> 0)
> logit(LOG_ERR, errno, "Can't bind RSRR socket");
>
> if (register_input_handler(rsrr_socket,rsrr_read) < 0)
> Index: usr.sbin/npppctl/npppctl.c
> ===================================================================
> RCS file: /data/src/openbsd/src/usr.sbin/npppctl/npppctl.c,v
> retrieving revision 1.6
> diff -u -p -r1.6 npppctl.c
> --- usr.sbin/npppctl/npppctl.c 5 Dec 2015 13:19:32 -0000 1.6
> +++ usr.sbin/npppctl/npppctl.c 6 Dec 2015 02:17:28 -0000
> @@ -106,7 +106,6 @@ main(int argc, char *argv[])
> err(EXIT_FAILURE, "socket");
> memset(&sun, 0, sizeof(sun));
> sun.sun_family = AF_UNIX;
> - sun.sun_len = sizeof(sun);
> strlcpy(sun.sun_path, npppd_ctlpath, sizeof(sun.sun_path));
> if (connect(ctlsock, (struct sockaddr *)&sun, sizeof(sun)) < 0)
> err(EXIT_FAILURE, "connect");
>
