bgpd, protability and sockaddr sa_len

classic Classic list List threaded Threaded
3 messages Options
Reply | Threaded
Open this post in threaded view
|

bgpd, protability and sockaddr sa_len

Claudio Jeker
Another diff to ease portability of bgpd. The sa_len field in struct
sockaddr does not exist on Linux so instead of using it pass a length to
the function (e.g. like bind(2) and connect(2) and do the same when
passing around struct sockaddr_storage in the listener case.
The remaining sa_len, sin_len, sin6_len and ss_len usages are in very
OpenBSD specific files.

--
:wq Claudio

Index: bgpd.h
===================================================================
RCS file: /cvs/src/usr.sbin/bgpd/bgpd.h,v
retrieving revision 1.369
diff -u -p -r1.369 bgpd.h
--- bgpd.h 15 Feb 2019 11:38:06 -0000 1.369
+++ bgpd.h 15 Feb 2019 13:51:28 -0000
@@ -220,11 +220,12 @@ struct bgpd_addr {
 #define LISTENER_LISTENING 0x02
 
 struct listen_addr {
- TAILQ_ENTRY(listen_addr) entry;
- struct sockaddr_storage sa;
- int fd;
- enum reconf_action reconf;
- u_int8_t flags;
+ TAILQ_ENTRY(listen_addr) entry;
+ struct sockaddr_storage sa;
+ int fd;
+ enum reconf_action reconf;
+ socklen_t sa_len;
+ u_int8_t flags;
 };
 
 TAILQ_HEAD(listen_addrs, listen_addr);
@@ -1254,7 +1255,7 @@ int set_equal(const struct set_table
 /* util.c */
 const char *log_addr(const struct bgpd_addr *);
 const char *log_in6addr(const struct in6_addr *);
-const char *log_sockaddr(struct sockaddr *);
+const char *log_sockaddr(struct sockaddr *, socklen_t);
 const char *log_as(u_int32_t);
 const char *log_rd(u_int64_t);
 const char *log_ext_subtype(u_int8_t, u_int8_t);
@@ -1288,7 +1289,7 @@ int aid2afi(u_int8_t, u_int16_t *, u_i
 int afi2aid(u_int16_t, u_int8_t, u_int8_t *);
 sa_family_t aid2af(u_int8_t);
 int af2aid(sa_family_t, u_int8_t, u_int8_t *);
-struct sockaddr *addr2sa(struct bgpd_addr *, u_int16_t);
+struct sockaddr *addr2sa(struct bgpd_addr *, u_int16_t, socklen_t *);
 void sa2addr(struct sockaddr *, struct bgpd_addr *);
 uint64_t ift2ifm(uint8_t);
 const char * get_media_descr(uint64_t);
Index: config.c
===================================================================
RCS file: /cvs/src/usr.sbin/bgpd/config.c,v
retrieving revision 1.81
diff -u -p -r1.81 config.c
--- config.c 12 Feb 2019 09:00:21 -0000 1.81
+++ config.c 15 Feb 2019 13:43:42 -0000
@@ -397,7 +397,7 @@ prepare_listeners(struct bgpd_config *co
  la->fd = -1;
  la->flags = DEFAULT_LISTENER;
  la->reconf = RECONF_REINIT;
- la->sa.ss_len = sizeof(struct sockaddr_in);
+ la->sa_len = sizeof(struct sockaddr_in);
  ((struct sockaddr_in *)&la->sa)->sin_family = AF_INET;
  ((struct sockaddr_in *)&la->sa)->sin_addr.s_addr =
     htonl(INADDR_ANY);
@@ -409,7 +409,7 @@ prepare_listeners(struct bgpd_config *co
  la->fd = -1;
  la->flags = DEFAULT_LISTENER;
  la->reconf = RECONF_REINIT;
- la->sa.ss_len = sizeof(struct sockaddr_in6);
+ la->sa_len = sizeof(struct sockaddr_in6);
  ((struct sockaddr_in6 *)&la->sa)->sin6_family = AF_INET6;
  ((struct sockaddr_in6 *)&la->sa)->sin6_port = htons(BGP_PORT);
  TAILQ_INSERT_TAIL(conf->listen_addrs, la, entry);
@@ -437,24 +437,25 @@ prepare_listeners(struct bgpd_config *co
     &opt, sizeof(opt)) == -1)
  fatal("setsockopt SO_REUSEADDR");
 
- if (bind(la->fd, (struct sockaddr *)&la->sa, la->sa.ss_len) ==
+ if (bind(la->fd, (struct sockaddr *)&la->sa, la->sa_len) ==
     -1) {
  switch (la->sa.ss_family) {
  case AF_INET:
  log_warn("cannot bind to %s:%u",
-    log_sockaddr((struct sockaddr *)&la->sa),
-    ntohs(((struct sockaddr_in *)
+    log_sockaddr((struct sockaddr *)&la->sa,
+    la->sa_len), ntohs(((struct sockaddr_in *)
     &la->sa)->sin_port));
  break;
  case AF_INET6:
  log_warn("cannot bind to [%s]:%u",
-    log_sockaddr((struct sockaddr *)&la->sa),
-    ntohs(((struct sockaddr_in6 *)
+    log_sockaddr((struct sockaddr *)&la->sa,
+    la->sa_len), ntohs(((struct sockaddr_in6 *)
     &la->sa)->sin6_port));
  break;
  default:
  log_warn("cannot bind to %s",
-    log_sockaddr((struct sockaddr *)&la->sa));
+    log_sockaddr((struct sockaddr *)&la->sa,
+    la->sa_len));
  break;
  }
  close(la->fd);
Index: logmsg.c
===================================================================
RCS file: /cvs/src/usr.sbin/bgpd/logmsg.c,v
retrieving revision 1.3
diff -u -p -r1.3 logmsg.c
--- logmsg.c 28 May 2017 20:14:15 -0000 1.3
+++ logmsg.c 15 Feb 2019 13:34:46 -0000
@@ -198,13 +198,13 @@ log_notification(const struct peer *peer
 }
 
 void
-log_conn_attempt(const struct peer *peer, struct sockaddr *sa)
+log_conn_attempt(const struct peer *peer, struct sockaddr *sa, socklen_t len)
 {
  char *p;
  const char *b;
 
  if (peer == NULL) { /* connection from non-peer, drop */
- b = log_sockaddr(sa);
+ b = log_sockaddr(sa, len);
  logit(LOG_INFO, "connection from non-peer %s refused", b);
  } else {
  /* only log if there is a chance that the session may come up */
Index: parse.y
===================================================================
RCS file: /cvs/src/usr.sbin/bgpd/parse.y,v
retrieving revision 1.374
diff -u -p -r1.374 parse.y
--- parse.y 15 Feb 2019 10:10:53 -0000 1.374
+++ parse.y 15 Feb 2019 13:58:06 -0000
@@ -578,13 +578,15 @@ conf_main : AS as4number {
  }
  | LISTEN ON address {
  struct listen_addr *la;
+ struct sockaddr *sa;
 
  if ((la = calloc(1, sizeof(struct listen_addr))) ==
     NULL)
  fatal("parse conf_main listen on calloc");
 
  la->fd = -1;
- memcpy(&la->sa, addr2sa(&$3, BGP_PORT), sizeof(la->sa));
+ sa = addr2sa(&$3, BGP_PORT, &la->sa_len);
+ memcpy(&la->sa, sa, la->sa_len);
  TAILQ_INSERT_TAIL(conf->listen_addrs, la, entry);
  }
  | FIBPRIORITY NUMBER {
Index: pfkey.c
===================================================================
RCS file: /cvs/src/usr.sbin/bgpd/pfkey.c,v
retrieving revision 1.52
diff -u -p -r1.52 pfkey.c
--- pfkey.c 20 Sep 2018 11:06:04 -0000 1.52
+++ pfkey.c 15 Feb 2019 13:54:36 -0000
@@ -76,6 +76,7 @@ pfkey_send(int sd, uint8_t satype, uint8
  int iov_cnt;
  struct sockaddr_storage ssrc, sdst, speer, smask, dmask;
  struct sockaddr *saptr;
+ socklen_t salen;
 
  if (!pid)
  pid = getpid();
@@ -83,8 +84,8 @@ pfkey_send(int sd, uint8_t satype, uint8
  /* we need clean sockaddr... no ports set */
  bzero(&ssrc, sizeof(ssrc));
  bzero(&smask, sizeof(smask));
- if ((saptr = addr2sa(src, 0)))
- memcpy(&ssrc, saptr, sizeof(ssrc));
+ if ((saptr = addr2sa(src, 0, &salen)))
+ memcpy(&ssrc, saptr, salen);
  switch (src->aid) {
  case AID_INET:
  memset(&((struct sockaddr_in *)&smask)->sin_addr, 0xff, 32/8);
@@ -104,8 +105,8 @@ pfkey_send(int sd, uint8_t satype, uint8
 
  bzero(&sdst, sizeof(sdst));
  bzero(&dmask, sizeof(dmask));
- if ((saptr = addr2sa(dst, 0)))
- memcpy(&sdst, saptr, sizeof(sdst));
+ if ((saptr = addr2sa(dst, 0, &salen)))
+ memcpy(&sdst, saptr, salen);
  switch (dst->aid) {
  case AID_INET:
  memset(&((struct sockaddr_in *)&dmask)->sin_addr, 0xff, 32/8);
Index: printconf.c
===================================================================
RCS file: /cvs/src/usr.sbin/bgpd/printconf.c,v
retrieving revision 1.129
diff -u -p -r1.129 printconf.c
--- printconf.c 12 Feb 2019 09:00:56 -0000 1.129
+++ printconf.c 15 Feb 2019 13:45:34 -0000
@@ -367,7 +367,7 @@ print_mainconf(struct bgpd_config *conf)
 
  TAILQ_FOREACH(la, conf->listen_addrs, entry)
  printf("listen on %s\n",
-    log_sockaddr((struct sockaddr *)&la->sa));
+    log_sockaddr((struct sockaddr *)&la->sa, la->sa_len));
 
  if (conf->flags & BGPD_FLAG_NEXTHOP_BGP)
  printf("nexthop qualify via bgp\n");
Index: session.c
===================================================================
RCS file: /cvs/src/usr.sbin/bgpd/session.c,v
retrieving revision 1.372
diff -u -p -r1.372 session.c
--- session.c 15 Feb 2019 11:38:06 -0000 1.372
+++ session.c 15 Feb 2019 13:53:32 -0000
@@ -145,7 +145,8 @@ setup_listeners(u_int *la_cnt)
 
  if (la->fd == -1) {
  log_warn("cannot establish listener on %s: invalid fd",
-    log_sockaddr((struct sockaddr *)&la->sa));
+    log_sockaddr((struct sockaddr *)&la->sa,
+    la->sa_len));
  continue;
  }
 
@@ -179,7 +180,7 @@ setup_listeners(u_int *la_cnt)
  la->flags |= LISTENER_LISTENING;
 
  log_info("listening on %s",
-    log_sockaddr((struct sockaddr *)&la->sa));
+    log_sockaddr((struct sockaddr *)&la->sa, la->sa_len));
  }
 
  *la_cnt = cnt;
@@ -1087,7 +1088,7 @@ open:
  /* then do part of the open dance */
  goto open;
  } else {
- log_conn_attempt(p, (struct sockaddr *)&cliaddr);
+ log_conn_attempt(p, (struct sockaddr *)&cliaddr, len);
  close(connfd);
  }
 }
@@ -1097,6 +1098,7 @@ session_connect(struct peer *peer)
 {
  int opt = 1;
  struct sockaddr *sa;
+ socklen_t sa_len;
 
  /*
  * we do not need the overcomplicated collision detection RFC 1771
@@ -1137,8 +1139,8 @@ session_connect(struct peer *peer)
  peer->wbuf.fd = peer->fd;
 
  /* if update source is set we need to bind() */
- if ((sa = addr2sa(&peer->conf.local_addr, 0)) != NULL) {
- if (bind(peer->fd, sa, sa->sa_len) == -1) {
+ if ((sa = addr2sa(&peer->conf.local_addr, 0, &sa_len)) != NULL) {
+ if (bind(peer->fd, sa, sa_len) == -1) {
  log_peer_warn(&peer->conf, "session_connect bind");
  bgp_fsm(peer, EVNT_CON_OPENFAIL);
  return (-1);
@@ -1150,8 +1152,8 @@ session_connect(struct peer *peer)
  return (-1);
  }
 
- sa = addr2sa(&peer->conf.remote_addr, BGP_PORT);
- if (connect(peer->fd, sa, sa->sa_len) == -1) {
+ sa = addr2sa(&peer->conf.remote_addr, BGP_PORT, &sa_len);
+ if (connect(peer->fd, sa, sa_len) == -1) {
  if (errno != EINPROGRESS) {
  if (errno != peer->lasterr)
  log_peer_warn(&peer->conf, "connect");
@@ -2695,7 +2697,7 @@ session_dispatch_imsg(struct imsgbuf *ib
  log_warnx("expected to receive fd for "
     "%s but didn't receive any",
     log_sockaddr((struct sockaddr *)
-    &nla->sa));
+    &nla->sa, nla->sa_len));
 
  la = calloc(1, sizeof(struct listen_addr));
  if (la == NULL)
@@ -2777,8 +2779,8 @@ session_dispatch_imsg(struct imsgbuf *ib
  nla = TAILQ_NEXT(la, entry);
  if (la->reconf == RECONF_NONE) {
  log_info("not listening on %s any more",
-    log_sockaddr(
-    (struct sockaddr *)&la->sa));
+    log_sockaddr((struct sockaddr *)
+    &la->sa, la->sa_len));
  TAILQ_REMOVE(conf->listen_addrs, la,
     entry);
  close(la->fd);
Index: session.h
===================================================================
RCS file: /cvs/src/usr.sbin/bgpd/session.h,v
retrieving revision 1.129
diff -u -p -r1.129 session.h
--- session.h 11 Feb 2019 15:44:25 -0000 1.129
+++ session.h 15 Feb 2019 13:35:11 -0000
@@ -264,7 +264,8 @@ void log_statechange(struct peer *, en
     enum session_events);
 void log_notification(const struct peer *, u_int8_t, u_int8_t,
     u_char *, u_int16_t, const char *);
-void log_conn_attempt(const struct peer *, struct sockaddr *);
+void log_conn_attempt(const struct peer *, struct sockaddr *,
+    socklen_t);
 
 /* mrt.c */
 void mrt_dump_bgp_msg(struct mrt *, void *, u_int16_t,
Index: util.c
===================================================================
RCS file: /cvs/src/usr.sbin/bgpd/util.c,v
retrieving revision 1.43
diff -u -p -r1.43 util.c
--- util.c 15 Feb 2019 09:55:21 -0000 1.43
+++ util.c 15 Feb 2019 13:55:38 -0000
@@ -74,7 +74,6 @@ log_in6addr(const struct in6_addr *addr)
  u_int16_t tmp16;
 
  bzero(&sa_in6, sizeof(sa_in6));
- sa_in6.sin6_len = sizeof(sa_in6);
  sa_in6.sin6_family = AF_INET6;
  memcpy(&sa_in6.sin6_addr, addr, sizeof(sa_in6.sin6_addr));
 
@@ -87,15 +86,15 @@ log_in6addr(const struct in6_addr *addr)
  sa_in6.sin6_addr.s6_addr[3] = 0;
  }
 
- return (log_sockaddr((struct sockaddr *)&sa_in6));
+ return (log_sockaddr((struct sockaddr *)&sa_in6, sizeof(sa_in6)));
 }
 
 const char *
-log_sockaddr(struct sockaddr *sa)
+log_sockaddr(struct sockaddr *sa, socklen_t len)
 {
  static char buf[NI_MAXHOST];
 
- if (getnameinfo(sa, sa->sa_len, buf, sizeof(buf), NULL, 0,
+ if (getnameinfo(sa, len, buf, sizeof(buf), NULL, 0,
     NI_NUMERICHOST))
  return ("(unknown)");
  else
@@ -839,7 +838,7 @@ af2aid(sa_family_t af, u_int8_t safi, u_
 }
 
 struct sockaddr *
-addr2sa(struct bgpd_addr *addr, u_int16_t port)
+addr2sa(struct bgpd_addr *addr, u_int16_t port, socklen_t *len)
 {
  static struct sockaddr_storage ss;
  struct sockaddr_in *sa_in = (struct sockaddr_in *)&ss;
@@ -852,17 +851,17 @@ addr2sa(struct bgpd_addr *addr, u_int16_
  switch (addr->aid) {
  case AID_INET:
  sa_in->sin_family = AF_INET;
- sa_in->sin_len = sizeof(struct sockaddr_in);
  sa_in->sin_addr.s_addr = addr->v4.s_addr;
  sa_in->sin_port = htons(port);
+ *len = sizeof(struct sockaddr_in);
  break;
  case AID_INET6:
  sa_in6->sin6_family = AF_INET6;
- sa_in6->sin6_len = sizeof(struct sockaddr_in6);
  memcpy(&sa_in6->sin6_addr, &addr->v6,
     sizeof(sa_in6->sin6_addr));
  sa_in6->sin6_port = htons(port);
  sa_in6->sin6_scope_id = addr->scope_id;
+ *len = sizeof(struct sockaddr_in6);
  break;
  }
 

Reply | Threaded
Open this post in threaded view
|

Re: bgpd, protability and sockaddr sa_len

Sebastian Benoit-3
ok

Claudio Jeker([hidden email]) on 2019.02.15 15:07:15 +0100:

> Another diff to ease portability of bgpd. The sa_len field in struct
> sockaddr does not exist on Linux so instead of using it pass a length to
> the function (e.g. like bind(2) and connect(2) and do the same when
> passing around struct sockaddr_storage in the listener case.
> The remaining sa_len, sin_len, sin6_len and ss_len usages are in very
> OpenBSD specific files.
>
> --
> :wq Claudio
>
> Index: bgpd.h
> ===================================================================
> RCS file: /cvs/src/usr.sbin/bgpd/bgpd.h,v
> retrieving revision 1.369
> diff -u -p -r1.369 bgpd.h
> --- bgpd.h 15 Feb 2019 11:38:06 -0000 1.369
> +++ bgpd.h 15 Feb 2019 13:51:28 -0000
> @@ -220,11 +220,12 @@ struct bgpd_addr {
>  #define LISTENER_LISTENING 0x02
>  
>  struct listen_addr {
> - TAILQ_ENTRY(listen_addr) entry;
> - struct sockaddr_storage sa;
> - int fd;
> - enum reconf_action reconf;
> - u_int8_t flags;
> + TAILQ_ENTRY(listen_addr) entry;
> + struct sockaddr_storage sa;
> + int fd;
> + enum reconf_action reconf;
> + socklen_t sa_len;
> + u_int8_t flags;
>  };
>  
>  TAILQ_HEAD(listen_addrs, listen_addr);
> @@ -1254,7 +1255,7 @@ int set_equal(const struct set_table
>  /* util.c */
>  const char *log_addr(const struct bgpd_addr *);
>  const char *log_in6addr(const struct in6_addr *);
> -const char *log_sockaddr(struct sockaddr *);
> +const char *log_sockaddr(struct sockaddr *, socklen_t);
>  const char *log_as(u_int32_t);
>  const char *log_rd(u_int64_t);
>  const char *log_ext_subtype(u_int8_t, u_int8_t);
> @@ -1288,7 +1289,7 @@ int aid2afi(u_int8_t, u_int16_t *, u_i
>  int afi2aid(u_int16_t, u_int8_t, u_int8_t *);
>  sa_family_t aid2af(u_int8_t);
>  int af2aid(sa_family_t, u_int8_t, u_int8_t *);
> -struct sockaddr *addr2sa(struct bgpd_addr *, u_int16_t);
> +struct sockaddr *addr2sa(struct bgpd_addr *, u_int16_t, socklen_t *);
>  void sa2addr(struct sockaddr *, struct bgpd_addr *);
>  uint64_t ift2ifm(uint8_t);
>  const char * get_media_descr(uint64_t);
> Index: config.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/bgpd/config.c,v
> retrieving revision 1.81
> diff -u -p -r1.81 config.c
> --- config.c 12 Feb 2019 09:00:21 -0000 1.81
> +++ config.c 15 Feb 2019 13:43:42 -0000
> @@ -397,7 +397,7 @@ prepare_listeners(struct bgpd_config *co
>   la->fd = -1;
>   la->flags = DEFAULT_LISTENER;
>   la->reconf = RECONF_REINIT;
> - la->sa.ss_len = sizeof(struct sockaddr_in);
> + la->sa_len = sizeof(struct sockaddr_in);
>   ((struct sockaddr_in *)&la->sa)->sin_family = AF_INET;
>   ((struct sockaddr_in *)&la->sa)->sin_addr.s_addr =
>      htonl(INADDR_ANY);
> @@ -409,7 +409,7 @@ prepare_listeners(struct bgpd_config *co
>   la->fd = -1;
>   la->flags = DEFAULT_LISTENER;
>   la->reconf = RECONF_REINIT;
> - la->sa.ss_len = sizeof(struct sockaddr_in6);
> + la->sa_len = sizeof(struct sockaddr_in6);
>   ((struct sockaddr_in6 *)&la->sa)->sin6_family = AF_INET6;
>   ((struct sockaddr_in6 *)&la->sa)->sin6_port = htons(BGP_PORT);
>   TAILQ_INSERT_TAIL(conf->listen_addrs, la, entry);
> @@ -437,24 +437,25 @@ prepare_listeners(struct bgpd_config *co
>      &opt, sizeof(opt)) == -1)
>   fatal("setsockopt SO_REUSEADDR");
>  
> - if (bind(la->fd, (struct sockaddr *)&la->sa, la->sa.ss_len) ==
> + if (bind(la->fd, (struct sockaddr *)&la->sa, la->sa_len) ==
>      -1) {
>   switch (la->sa.ss_family) {
>   case AF_INET:
>   log_warn("cannot bind to %s:%u",
> -    log_sockaddr((struct sockaddr *)&la->sa),
> -    ntohs(((struct sockaddr_in *)
> +    log_sockaddr((struct sockaddr *)&la->sa,
> +    la->sa_len), ntohs(((struct sockaddr_in *)
>      &la->sa)->sin_port));
>   break;
>   case AF_INET6:
>   log_warn("cannot bind to [%s]:%u",
> -    log_sockaddr((struct sockaddr *)&la->sa),
> -    ntohs(((struct sockaddr_in6 *)
> +    log_sockaddr((struct sockaddr *)&la->sa,
> +    la->sa_len), ntohs(((struct sockaddr_in6 *)
>      &la->sa)->sin6_port));
>   break;
>   default:
>   log_warn("cannot bind to %s",
> -    log_sockaddr((struct sockaddr *)&la->sa));
> +    log_sockaddr((struct sockaddr *)&la->sa,
> +    la->sa_len));
>   break;
>   }
>   close(la->fd);
> Index: logmsg.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/bgpd/logmsg.c,v
> retrieving revision 1.3
> diff -u -p -r1.3 logmsg.c
> --- logmsg.c 28 May 2017 20:14:15 -0000 1.3
> +++ logmsg.c 15 Feb 2019 13:34:46 -0000
> @@ -198,13 +198,13 @@ log_notification(const struct peer *peer
>  }
>  
>  void
> -log_conn_attempt(const struct peer *peer, struct sockaddr *sa)
> +log_conn_attempt(const struct peer *peer, struct sockaddr *sa, socklen_t len)
>  {
>   char *p;
>   const char *b;
>  
>   if (peer == NULL) { /* connection from non-peer, drop */
> - b = log_sockaddr(sa);
> + b = log_sockaddr(sa, len);
>   logit(LOG_INFO, "connection from non-peer %s refused", b);
>   } else {
>   /* only log if there is a chance that the session may come up */
> Index: parse.y
> ===================================================================
> RCS file: /cvs/src/usr.sbin/bgpd/parse.y,v
> retrieving revision 1.374
> diff -u -p -r1.374 parse.y
> --- parse.y 15 Feb 2019 10:10:53 -0000 1.374
> +++ parse.y 15 Feb 2019 13:58:06 -0000
> @@ -578,13 +578,15 @@ conf_main : AS as4number {
>   }
>   | LISTEN ON address {
>   struct listen_addr *la;
> + struct sockaddr *sa;
>  
>   if ((la = calloc(1, sizeof(struct listen_addr))) ==
>      NULL)
>   fatal("parse conf_main listen on calloc");
>  
>   la->fd = -1;
> - memcpy(&la->sa, addr2sa(&$3, BGP_PORT), sizeof(la->sa));
> + sa = addr2sa(&$3, BGP_PORT, &la->sa_len);
> + memcpy(&la->sa, sa, la->sa_len);
>   TAILQ_INSERT_TAIL(conf->listen_addrs, la, entry);
>   }
>   | FIBPRIORITY NUMBER {
> Index: pfkey.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/bgpd/pfkey.c,v
> retrieving revision 1.52
> diff -u -p -r1.52 pfkey.c
> --- pfkey.c 20 Sep 2018 11:06:04 -0000 1.52
> +++ pfkey.c 15 Feb 2019 13:54:36 -0000
> @@ -76,6 +76,7 @@ pfkey_send(int sd, uint8_t satype, uint8
>   int iov_cnt;
>   struct sockaddr_storage ssrc, sdst, speer, smask, dmask;
>   struct sockaddr *saptr;
> + socklen_t salen;
>  
>   if (!pid)
>   pid = getpid();
> @@ -83,8 +84,8 @@ pfkey_send(int sd, uint8_t satype, uint8
>   /* we need clean sockaddr... no ports set */
>   bzero(&ssrc, sizeof(ssrc));
>   bzero(&smask, sizeof(smask));
> - if ((saptr = addr2sa(src, 0)))
> - memcpy(&ssrc, saptr, sizeof(ssrc));
> + if ((saptr = addr2sa(src, 0, &salen)))
> + memcpy(&ssrc, saptr, salen);
>   switch (src->aid) {
>   case AID_INET:
>   memset(&((struct sockaddr_in *)&smask)->sin_addr, 0xff, 32/8);
> @@ -104,8 +105,8 @@ pfkey_send(int sd, uint8_t satype, uint8
>  
>   bzero(&sdst, sizeof(sdst));
>   bzero(&dmask, sizeof(dmask));
> - if ((saptr = addr2sa(dst, 0)))
> - memcpy(&sdst, saptr, sizeof(sdst));
> + if ((saptr = addr2sa(dst, 0, &salen)))
> + memcpy(&sdst, saptr, salen);
>   switch (dst->aid) {
>   case AID_INET:
>   memset(&((struct sockaddr_in *)&dmask)->sin_addr, 0xff, 32/8);
> Index: printconf.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/bgpd/printconf.c,v
> retrieving revision 1.129
> diff -u -p -r1.129 printconf.c
> --- printconf.c 12 Feb 2019 09:00:56 -0000 1.129
> +++ printconf.c 15 Feb 2019 13:45:34 -0000
> @@ -367,7 +367,7 @@ print_mainconf(struct bgpd_config *conf)
>  
>   TAILQ_FOREACH(la, conf->listen_addrs, entry)
>   printf("listen on %s\n",
> -    log_sockaddr((struct sockaddr *)&la->sa));
> +    log_sockaddr((struct sockaddr *)&la->sa, la->sa_len));
>  
>   if (conf->flags & BGPD_FLAG_NEXTHOP_BGP)
>   printf("nexthop qualify via bgp\n");
> Index: session.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/bgpd/session.c,v
> retrieving revision 1.372
> diff -u -p -r1.372 session.c
> --- session.c 15 Feb 2019 11:38:06 -0000 1.372
> +++ session.c 15 Feb 2019 13:53:32 -0000
> @@ -145,7 +145,8 @@ setup_listeners(u_int *la_cnt)
>  
>   if (la->fd == -1) {
>   log_warn("cannot establish listener on %s: invalid fd",
> -    log_sockaddr((struct sockaddr *)&la->sa));
> +    log_sockaddr((struct sockaddr *)&la->sa,
> +    la->sa_len));
>   continue;
>   }
>  
> @@ -179,7 +180,7 @@ setup_listeners(u_int *la_cnt)
>   la->flags |= LISTENER_LISTENING;
>  
>   log_info("listening on %s",
> -    log_sockaddr((struct sockaddr *)&la->sa));
> +    log_sockaddr((struct sockaddr *)&la->sa, la->sa_len));
>   }
>  
>   *la_cnt = cnt;
> @@ -1087,7 +1088,7 @@ open:
>   /* then do part of the open dance */
>   goto open;
>   } else {
> - log_conn_attempt(p, (struct sockaddr *)&cliaddr);
> + log_conn_attempt(p, (struct sockaddr *)&cliaddr, len);
>   close(connfd);
>   }
>  }
> @@ -1097,6 +1098,7 @@ session_connect(struct peer *peer)
>  {
>   int opt = 1;
>   struct sockaddr *sa;
> + socklen_t sa_len;
>  
>   /*
>   * we do not need the overcomplicated collision detection RFC 1771
> @@ -1137,8 +1139,8 @@ session_connect(struct peer *peer)
>   peer->wbuf.fd = peer->fd;
>  
>   /* if update source is set we need to bind() */
> - if ((sa = addr2sa(&peer->conf.local_addr, 0)) != NULL) {
> - if (bind(peer->fd, sa, sa->sa_len) == -1) {
> + if ((sa = addr2sa(&peer->conf.local_addr, 0, &sa_len)) != NULL) {
> + if (bind(peer->fd, sa, sa_len) == -1) {
>   log_peer_warn(&peer->conf, "session_connect bind");
>   bgp_fsm(peer, EVNT_CON_OPENFAIL);
>   return (-1);
> @@ -1150,8 +1152,8 @@ session_connect(struct peer *peer)
>   return (-1);
>   }
>  
> - sa = addr2sa(&peer->conf.remote_addr, BGP_PORT);
> - if (connect(peer->fd, sa, sa->sa_len) == -1) {
> + sa = addr2sa(&peer->conf.remote_addr, BGP_PORT, &sa_len);
> + if (connect(peer->fd, sa, sa_len) == -1) {
>   if (errno != EINPROGRESS) {
>   if (errno != peer->lasterr)
>   log_peer_warn(&peer->conf, "connect");
> @@ -2695,7 +2697,7 @@ session_dispatch_imsg(struct imsgbuf *ib
>   log_warnx("expected to receive fd for "
>      "%s but didn't receive any",
>      log_sockaddr((struct sockaddr *)
> -    &nla->sa));
> +    &nla->sa, nla->sa_len));
>  
>   la = calloc(1, sizeof(struct listen_addr));
>   if (la == NULL)
> @@ -2777,8 +2779,8 @@ session_dispatch_imsg(struct imsgbuf *ib
>   nla = TAILQ_NEXT(la, entry);
>   if (la->reconf == RECONF_NONE) {
>   log_info("not listening on %s any more",
> -    log_sockaddr(
> -    (struct sockaddr *)&la->sa));
> +    log_sockaddr((struct sockaddr *)
> +    &la->sa, la->sa_len));
>   TAILQ_REMOVE(conf->listen_addrs, la,
>      entry);
>   close(la->fd);
> Index: session.h
> ===================================================================
> RCS file: /cvs/src/usr.sbin/bgpd/session.h,v
> retrieving revision 1.129
> diff -u -p -r1.129 session.h
> --- session.h 11 Feb 2019 15:44:25 -0000 1.129
> +++ session.h 15 Feb 2019 13:35:11 -0000
> @@ -264,7 +264,8 @@ void log_statechange(struct peer *, en
>      enum session_events);
>  void log_notification(const struct peer *, u_int8_t, u_int8_t,
>      u_char *, u_int16_t, const char *);
> -void log_conn_attempt(const struct peer *, struct sockaddr *);
> +void log_conn_attempt(const struct peer *, struct sockaddr *,
> +    socklen_t);
>  
>  /* mrt.c */
>  void mrt_dump_bgp_msg(struct mrt *, void *, u_int16_t,
> Index: util.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/bgpd/util.c,v
> retrieving revision 1.43
> diff -u -p -r1.43 util.c
> --- util.c 15 Feb 2019 09:55:21 -0000 1.43
> +++ util.c 15 Feb 2019 13:55:38 -0000
> @@ -74,7 +74,6 @@ log_in6addr(const struct in6_addr *addr)
>   u_int16_t tmp16;
>  
>   bzero(&sa_in6, sizeof(sa_in6));
> - sa_in6.sin6_len = sizeof(sa_in6);
>   sa_in6.sin6_family = AF_INET6;
>   memcpy(&sa_in6.sin6_addr, addr, sizeof(sa_in6.sin6_addr));
>  
> @@ -87,15 +86,15 @@ log_in6addr(const struct in6_addr *addr)
>   sa_in6.sin6_addr.s6_addr[3] = 0;
>   }
>  
> - return (log_sockaddr((struct sockaddr *)&sa_in6));
> + return (log_sockaddr((struct sockaddr *)&sa_in6, sizeof(sa_in6)));
>  }
>  
>  const char *
> -log_sockaddr(struct sockaddr *sa)
> +log_sockaddr(struct sockaddr *sa, socklen_t len)
>  {
>   static char buf[NI_MAXHOST];
>  
> - if (getnameinfo(sa, sa->sa_len, buf, sizeof(buf), NULL, 0,
> + if (getnameinfo(sa, len, buf, sizeof(buf), NULL, 0,
>      NI_NUMERICHOST))
>   return ("(unknown)");
>   else
> @@ -839,7 +838,7 @@ af2aid(sa_family_t af, u_int8_t safi, u_
>  }
>  
>  struct sockaddr *
> -addr2sa(struct bgpd_addr *addr, u_int16_t port)
> +addr2sa(struct bgpd_addr *addr, u_int16_t port, socklen_t *len)
>  {
>   static struct sockaddr_storage ss;
>   struct sockaddr_in *sa_in = (struct sockaddr_in *)&ss;
> @@ -852,17 +851,17 @@ addr2sa(struct bgpd_addr *addr, u_int16_
>   switch (addr->aid) {
>   case AID_INET:
>   sa_in->sin_family = AF_INET;
> - sa_in->sin_len = sizeof(struct sockaddr_in);
>   sa_in->sin_addr.s_addr = addr->v4.s_addr;
>   sa_in->sin_port = htons(port);
> + *len = sizeof(struct sockaddr_in);
>   break;
>   case AID_INET6:
>   sa_in6->sin6_family = AF_INET6;
> - sa_in6->sin6_len = sizeof(struct sockaddr_in6);
>   memcpy(&sa_in6->sin6_addr, &addr->v6,
>      sizeof(sa_in6->sin6_addr));
>   sa_in6->sin6_port = htons(port);
>   sa_in6->sin6_scope_id = addr->scope_id;
> + *len = sizeof(struct sockaddr_in6);
>   break;
>   }
>  
>

Reply | Threaded
Open this post in threaded view
|

Re: bgpd, protability and sockaddr sa_len

William Ahern-2
In reply to this post by Claudio Jeker
On Fri, Feb 15, 2019 at 03:07:15PM +0100, Claudio Jeker wrote:

> Another diff to ease portability of bgpd. The sa_len field in struct
> sockaddr does not exist on Linux so instead of using it pass a length to
> the function (e.g. like bind(2) and connect(2) and do the same when
> passing around struct sockaddr_storage in the listener case.
> The remaining sa_len, sin_len, sin6_len and ss_len usages are in very
> OpenBSD specific files.
>
> --
> :wq Claudio
>
> Index: bgpd.h
> ===================================================================
> RCS file: /cvs/src/usr.sbin/bgpd/bgpd.h,v
> retrieving revision 1.369
> diff -u -p -r1.369 bgpd.h
> --- bgpd.h 15 Feb 2019 11:38:06 -0000 1.369
> +++ bgpd.h 15 Feb 2019 13:51:28 -0000
> @@ -220,11 +220,12 @@ struct bgpd_addr {
>  #define LISTENER_LISTENING 0x02
>  
>  struct listen_addr {
> - TAILQ_ENTRY(listen_addr) entry;
> - struct sockaddr_storage sa;
> - int fd;
> - enum reconf_action reconf;
> - u_int8_t flags;
> + TAILQ_ENTRY(listen_addr) entry;
> + struct sockaddr_storage sa;
> + int fd;
> + enum reconf_action reconf;
> + socklen_t sa_len;
> + u_int8_t flags;
>  };

What's the use of maintaining and passing around sa_len if the sa member is
a fixed size? (Well, other than being a more straightforward patch.)

AFAIK the only variably sized sockaddr structure is sockaddr_un. Domain
socket paths can be longer than what sockaddr_un (or sockaddr_storage) can
nominally fit, but if the sa member is a fixed size then it's irrelevant and
you can always derive the sa object size from .sa_family or .sun_family +
.sun_path.