Jiri Pirko (3): teamd: fix incorrect usage of sizeof in __str_sockaddr fix strncmp len in ifname2ifindex teamd: tipc: use TIPC_MAX_*_NAME for buffers and check len
include/private/misc.h | 2 +- teamd/teamd_link_watch.c | 5 ++--- teamd/teamd_link_watch.h | 2 +- teamd/teamd_lw_arp_ping.c | 2 +- teamd/teamd_lw_nsna_ping.c | 5 +++-- teamd/teamd_lw_tipc.c | 10 +++++----- 6 files changed, 13 insertions(+), 13 deletions(-)
Sizeof will return the size of a pointer, which is not correct intended.
Introduced-by: 672ddfd506 ("teamd: move arp ping link watcher to a separate file") Signed-off-by: Jiri Pirko jiri@resnulli.us --- teamd/teamd_link_watch.c | 5 ++--- teamd/teamd_link_watch.h | 2 +- teamd/teamd_lw_arp_ping.c | 2 +- teamd/teamd_lw_nsna_ping.c | 5 +++-- 4 files changed, 7 insertions(+), 7 deletions(-)
diff --git a/teamd/teamd_link_watch.c b/teamd/teamd_link_watch.c index 55d81a9..9441bd6 100644 --- a/teamd/teamd_link_watch.c +++ b/teamd/teamd_link_watch.c @@ -64,13 +64,12 @@ int __set_sockaddr(struct sockaddr *sa, socklen_t sa_len, sa_family_t family,
char *__str_sockaddr(struct sockaddr *sa, socklen_t sa_len, sa_family_t family, - char buf[]) + char *buf, size_t buflen) { int err;
sa->sa_family = family; - err = getnameinfo(sa, sa_len, buf, sizeof(buf), - NULL, 0, NI_NUMERICHOST); + err = getnameinfo(sa, sa_len, buf, buflen, NULL, 0, NI_NUMERICHOST); if (err) { teamd_log_err("getnameinfo failed: %s", gai_strerror(err)); return NULL; diff --git a/teamd/teamd_link_watch.h b/teamd/teamd_link_watch.h index 8b76653..a4d441c 100644 --- a/teamd/teamd_link_watch.h +++ b/teamd/teamd_link_watch.h @@ -45,7 +45,7 @@ struct lw_psr_port_priv { int __set_sockaddr(struct sockaddr *sa, socklen_t sa_len, sa_family_t family, const char *hostname); char *__str_sockaddr(struct sockaddr *sa, socklen_t sa_len, sa_family_t family, - char buf[]); + char *buf, size_t buflen);
static inline bool teamd_link_watch_link_up_differs(struct lw_common_port_priv *common_ppriv, bool new_link_up) diff --git a/teamd/teamd_lw_arp_ping.c b/teamd/teamd_lw_arp_ping.c index 77d6de5..dc7515f 100644 --- a/teamd/teamd_lw_arp_ping.c +++ b/teamd/teamd_lw_arp_ping.c @@ -122,7 +122,7 @@ static char *str_in_addr(struct in_addr *addr)
memcpy(&sin.sin_addr, addr, sizeof(*addr)); return __str_sockaddr((struct sockaddr *) &sin, sizeof(sin), AF_INET, - buf); + buf, sizeof(buf)); }
static int lw_ap_sock_open(struct lw_psr_port_priv *psr_ppriv) diff --git a/teamd/teamd_lw_nsna_ping.c b/teamd/teamd_lw_nsna_ping.c index 4b9e52e..468143e 100644 --- a/teamd/teamd_lw_nsna_ping.c +++ b/teamd/teamd_lw_nsna_ping.c @@ -45,8 +45,9 @@ static int set_sockaddr_in6(struct sockaddr_in6 *sin6, const char *hostname) static char *str_sockaddr_in6(struct sockaddr_in6 *sin6) { static char buf[NI_MAXHOST]; - return __str_sockaddr((struct sockaddr *) sin6, - sizeof(*sin6), AF_INET6, buf); + + return __str_sockaddr((struct sockaddr *) sin6, sizeof(*sin6), AF_INET6, + buf, sizeof(buf)); }
struct lw_nsnap_port_priv {
On Wed, Aug 06, 2014 at 05:49:16PM +0200, Jiri Pirko wrote:
Sizeof will return the size of a pointer, which is not correct intended.
Introduced-by: 672ddfd506 ("teamd: move arp ping link watcher to a separate file") Signed-off-by: Jiri Pirko jiri@resnulli.us
teamd/teamd_link_watch.c | 5 ++--- teamd/teamd_link_watch.h | 2 +- teamd/teamd_lw_arp_ping.c | 2 +- teamd/teamd_lw_nsna_ping.c | 5 +++-- 4 files changed, 7 insertions(+), 7 deletions(-)
diff --git a/teamd/teamd_link_watch.c b/teamd/teamd_link_watch.c index 55d81a9..9441bd6 100644 --- a/teamd/teamd_link_watch.c +++ b/teamd/teamd_link_watch.c @@ -64,13 +64,12 @@ int __set_sockaddr(struct sockaddr *sa, socklen_t sa_len, sa_family_t family,
char *__str_sockaddr(struct sockaddr *sa, socklen_t sa_len, sa_family_t family,
char buf[])
char *buf, size_t buflen){ int err;
sa->sa_family = family;
- err = getnameinfo(sa, sa_len, buf, sizeof(buf),
NULL, 0, NI_NUMERICHOST);
- err = getnameinfo(sa, sa_len, buf, buflen, NULL, 0, NI_NUMERICHOST); if (err) { teamd_log_err("getnameinfo failed: %s", gai_strerror(err)); return NULL;
diff --git a/teamd/teamd_link_watch.h b/teamd/teamd_link_watch.h index 8b76653..a4d441c 100644 --- a/teamd/teamd_link_watch.h +++ b/teamd/teamd_link_watch.h @@ -45,7 +45,7 @@ struct lw_psr_port_priv { int __set_sockaddr(struct sockaddr *sa, socklen_t sa_len, sa_family_t family, const char *hostname); char *__str_sockaddr(struct sockaddr *sa, socklen_t sa_len, sa_family_t family,
char buf[]);
char *buf, size_t buflen);static inline bool teamd_link_watch_link_up_differs(struct lw_common_port_priv *common_ppriv, bool new_link_up) diff --git a/teamd/teamd_lw_arp_ping.c b/teamd/teamd_lw_arp_ping.c index 77d6de5..dc7515f 100644 --- a/teamd/teamd_lw_arp_ping.c +++ b/teamd/teamd_lw_arp_ping.c @@ -122,7 +122,7 @@ static char *str_in_addr(struct in_addr *addr)
memcpy(&sin.sin_addr, addr, sizeof(*addr)); return __str_sockaddr((struct sockaddr *) &sin, sizeof(sin), AF_INET,
buf);
buf, sizeof(buf));
It's ok to use sizeof() here since buf is static array.
}
static int lw_ap_sock_open(struct lw_psr_port_priv *psr_ppriv) diff --git a/teamd/teamd_lw_nsna_ping.c b/teamd/teamd_lw_nsna_ping.c index 4b9e52e..468143e 100644 --- a/teamd/teamd_lw_nsna_ping.c +++ b/teamd/teamd_lw_nsna_ping.c @@ -45,8 +45,9 @@ static int set_sockaddr_in6(struct sockaddr_in6 *sin6, const char *hostname) static char *str_sockaddr_in6(struct sockaddr_in6 *sin6) { static char buf[NI_MAXHOST];
- return __str_sockaddr((struct sockaddr *) sin6,
sizeof(*sin6), AF_INET6, buf);
- return __str_sockaddr((struct sockaddr *) sin6, sizeof(*sin6), AF_INET6,
buf, sizeof(buf));
Same here.
No functional change, so Acked-by: Flavio Leitner fbl@redhat.com
}
struct lw_nsnap_port_priv {
1.9.3
libteam mailing list libteam@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/libteam
Calling strncpy with a maximum size argument of 16 bytes on destination array "ifr.ifr_ifrn.ifrn_name" of size 16 bytes might leave the destination string unterminated.
Introduced-by: b9692cb90 ("teamd: generate team device names in case they are not specified") Signed-off-by: Jiri Pirko jiri@resnulli.us --- include/private/misc.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/include/private/misc.h b/include/private/misc.h index 5eae7ac..9814b45 100644 --- a/include/private/misc.h +++ b/include/private/misc.h @@ -88,7 +88,7 @@ static inline int ifname2ifindex(uint32_t *p_ifindex, char *ifname) if (sock == -1) return -errno; memset(&ifr, 0, sizeof(ifr)); - strncpy(ifr.ifr_name, ifname, IFNAMSIZ); + strncpy(ifr.ifr_name, ifname, IFNAMSIZ - 1); ret = ioctl(sock, SIOCGIFINDEX, &ifr); close(sock); if (ret == -1) {
On Wed, Aug 06, 2014 at 05:49:17PM +0200, Jiri Pirko wrote:
Calling strncpy with a maximum size argument of 16 bytes on destination array "ifr.ifr_ifrn.ifrn_name" of size 16 bytes might leave the destination string unterminated.
Introduced-by: b9692cb90 ("teamd: generate team device names in case they are not specified") Signed-off-by: Jiri Pirko jiri@resnulli.us
Acked-by: Flavio Leitner fbl@redhat.com
You might overrun the 255 byte fixed-size string "tipc_ppriv->bearer" by copying "tipc_bearer" without checking the length.
Introduced-by: 847046a5c7 ("teamd: add TIPC link watcher") Signed-off-by: Jiri Pirko jiri@resnulli.us --- teamd/teamd_lw_tipc.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/teamd/teamd_lw_tipc.c b/teamd/teamd_lw_tipc.c index 28d59e4..e86cd07 100644 --- a/teamd/teamd_lw_tipc.c +++ b/teamd/teamd_lw_tipc.c @@ -34,14 +34,14 @@
struct tipc_link { LIST_ENTRY(tipc_link) next; - char name[255]; + char name[TIPC_MAX_LINK_NAME]; bool up; };
struct lw_tipc_port_priv { struct lw_common_port_priv common; int topsrv_sock; - char bearer[255]; + char bearer[TIPC_MAX_BEARER_NAME]; LIST_HEAD(links, tipc_link) links; int active_links; }; @@ -56,8 +56,9 @@ static int lw_tipc_load_options(struct teamd_context *ctx, err = teamd_config_string_get(ctx, &tipc_bearer, "@.tipc_bearer", cpcookie); if (err) return -EINVAL; + if (strlen(tipc_bearer) >= TIPC_MAX_BEARER_NAME) + return -EINVAL; strcpy(tipc_ppriv->bearer, tipc_bearer); - return 0; }
@@ -100,7 +101,7 @@ link_up: static int lw_tipc_filter_events(struct lw_tipc_port_priv *tipc_ppriv, struct tipc_sioc_ln_req *lnr) { - char name[255]; + char name[TIPC_MAX_LINK_NAME]; char needle[24]; char *remote, *bearer;
@@ -225,7 +226,6 @@ static void lw_tipc_port_removed(struct teamd_context *ctx, close(tipc_ppriv->topsrv_sock); while (tipc_ppriv->links.lh_first != NULL) LIST_REMOVE(tipc_ppriv->links.lh_first, next); - }
int lw_tipc_state_bearer_get(struct teamd_context *ctx,
On Wed, Aug 06, 2014 at 05:49:18PM +0200, Jiri Pirko wrote:
You might overrun the 255 byte fixed-size string "tipc_ppriv->bearer" by copying "tipc_bearer" without checking the length.
Introduced-by: 847046a5c7 ("teamd: add TIPC link watcher") Signed-off-by: Jiri Pirko jiri@resnulli.us
Looks good to me.
Acked-by: Flavio Leitner fbl@redhat.com
libteam@lists.fedorahosted.org
-
Flavio Leitner -
Jiri Pirko