ctdb-common: Avoid Coverity 1697470/1697471

These are listed as: Memory - corruptions (OVERRUN)

While sockaddr_ll only has 8 octets for sll_addr, a sockaddr_storage
is being intentionally used here to provide additional space.  So, the
overrun is a technicality and there is no corruption.

However, the code is incorrect because it does overrun sll_addr[] in
sockaddr_ll.  The assignments into out-of-bounds indices also cause
the -O3 build fail.

So, use an extra variable to point to where sll_addr should start in a
sockaddr_storage and index from there.

Signed-off-by: Martin Schwenke <mschwenke@ddn.com>
Reviewed-by: Anoop C S <anoopcs@samba.org>

diff --git a/ctdb/common/system_socket.c b/ctdb/common/system_socket.c
index 97dd1b92341d7e948664615d487c89a6b9a0c1d7..6e97332e235849e55a272ee1784606d3fa0b3ccc 100644 (file)
--- a/ctdb/common/system_socket.c
+++ b/ctdb/common/system_socket.c
@@ -534,9 +534,12 @@ static int ip6_ll_multicast_build(struct sockaddr_ll *in,
        const uint8_t ethernet_multicast[ETH_ALEN] = {
                0x33, 0x33, 0x00, 0x00, 0x00, 0x01
        };
-       struct sockaddr_ll *out_sall = (struct sockaddr_ll *)out;
+       uint8_t *in_sll_addr = (uint8_t *)in + offsetof(struct sockaddr_ll,
+                                                       sll_addr);
+       uint8_t *out_sll_addr = (uint8_t *)out + offsetof(struct sockaddr_ll,
+                                                         sll_addr);
 
-       *out_sall = (struct sockaddr_ll) {
+       *(struct sockaddr_ll *)out = (struct sockaddr_ll) {
                .sll_family = AF_PACKET,
                .sll_halen = in->sll_halen,
                .sll_protocol = htons(ETH_P_IPV6),
@@ -548,7 +551,7 @@ static int ip6_ll_multicast_build(struct sockaddr_ll *in,
                if (SOCKADDR_LL_ADDR_LEN < sizeof(ethernet_multicast)) {
                        return EMSGSIZE;
                }
-               memcpy(&out_sall->sll_addr[0],
+               memcpy(&out_sll_addr[0],
                       ethernet_multicast,
                       sizeof(ethernet_multicast));
                break;
@@ -595,14 +598,12 @@ static int ip6_ll_multicast_build(struct sockaddr_ll *in,
                 * appear to be the last 4 octets of the IPv6
                 * multicast address (see all_nodes_ll_multicast).
                 */
-               memcpy(&out_sall->sll_addr[0],
-                      &in->sll_addr[0],
-                      in->sll_halen);
-               out_sall->sll_addr[6] = 0x60;
-               out_sall->sll_addr[16] = 0x00;
-               out_sall->sll_addr[17] = 0x00;
-               out_sall->sll_addr[18] = 0x00;
-               out_sall->sll_addr[19] = 0x01;
+               memcpy(&out_sll_addr[0], &in_sll_addr[0], in->sll_halen);
+               out_sll_addr[6] = 0x60;
+               out_sll_addr[16] = 0x00;
+               out_sll_addr[17] = 0x00;
+               out_sll_addr[18] = 0x00;
+               out_sll_addr[19] = 0x01;
                break;
        default:
                return EPROTONOSUPPORT;

diff --git a/ctdb/tests/src/system_socket_test.c b/ctdb/tests/src/system_socket_test.c
index 32eee673771a9c24dbd1fba759baf502c419d027..a22bd1e11bf5cbe845383a468c6d86842621a868 100644 (file)
--- a/ctdb/tests/src/system_socket_test.c
+++ b/ctdb/tests/src/system_socket_test.c
@@ -82,6 +82,8 @@ static void hwaddr_to_sockaddr_ll(const char *asc, struct sockaddr_ll *sall)
        char in[strlen(asc) + 1];
        char *t = NULL;
        char *tok = NULL;
+       uint8_t *sll_addr = (uint8_t *)sall + offsetof(struct sockaddr_ll,
+                                                      sll_addr);
 
        *sall = (struct sockaddr_ll) {
                .sll_family = AF_PACKET,
@@ -96,9 +98,10 @@ static void hwaddr_to_sockaddr_ll(const char *asc, struct sockaddr_ll *sall)
                char *end = NULL;
                unsigned long octet = strtoul(tok, &end, 16);
 
+               assert(sall->sll_halen < SOCKADDR_LL_ADDR_LEN);
                assert(end != NULL && *end == '\0');
                assert(octet <= 0xff);
-               sall->sll_addr[sall->sll_halen] = (uint8_t)octet;
+               sll_addr[sall->sll_halen] = (uint8_t)octet;
                sall->sll_halen++;
 
                t = NULL;