netfilter: ipset: Fix extension alignment

The data extensions in ipset lacked the proper memory alignment and
thus could lead to kernel crash on several architectures. Therefore
the structures have been reorganized and alignment attributes added
where needed. The patch was tested on armv7h by Gerhard Wiesinger and
on x86_64, sparc64 by Jozsef Kadlecsik.

Reported-by: Gerhard Wiesinger <lists@wiesinger.com>
Tested-by: Gerhard Wiesinger <lists@wiesinger.com>
Tested-by: Jozsef Kadlecsik <kadlec@blackhole.kfki.hu>
Signed-off-by: Jozsef Kadlecsik <kadlec@blackhole.kfki.hu>

diff --git a/include/linux/netfilter/ipset/ip_set.h b/include/linux/netfilter/ipset/ip_set.h
index 48bb01edcf3047bb70b70188430080c5970583a8..0e1f433cc4b78537c24e0e5f410894655d90aa0e 100644 (file)
--- a/include/linux/netfilter/ipset/ip_set.h
+++ b/include/linux/netfilter/ipset/ip_set.h
@@ -421,7 +421,7 @@ extern void ip_set_free(void *members);
 extern int ip_set_get_ipaddr4(struct nlattr *nla,  __be32 *ipaddr);
 extern int ip_set_get_ipaddr6(struct nlattr *nla, union nf_inet_addr *ipaddr);
 extern size_t ip_set_elem_len(struct ip_set *set, struct nlattr *tb[],
-                             size_t len);
+                             size_t len, size_t align);
 extern int ip_set_get_extensions(struct ip_set *set, struct nlattr *tb[],
                                 struct ip_set_ext *ext);
 

diff --git a/net/netfilter/ipset/ip_set_bitmap_gen.h b/net/netfilter/ipset/ip_set_bitmap_gen.h
index d05e759ed0fafe62ef299e6f097f0d18f08ec567..b0bc475f641e3b2c808f78f5be54283de6866d25 100644 (file)
--- a/net/netfilter/ipset/ip_set_bitmap_gen.h
+++ b/net/netfilter/ipset/ip_set_bitmap_gen.h
@@ -33,7 +33,7 @@
 #define mtype_gc               IPSET_TOKEN(MTYPE, _gc)
 #define mtype                  MTYPE
 
-#define get_ext(set, map, id)  ((map)->extensions + (set)->dsize * (id))
+#define get_ext(set, map, id)  ((map)->extensions + ((set)->dsize * (id)))
 
 static void
 mtype_gc_init(struct ip_set *set, void (*gc)(unsigned long ul_set))
@@ -67,12 +67,9 @@ mtype_destroy(struct ip_set *set)
                del_timer_sync(&map->gc);
 
        ip_set_free(map->members);
-       if (set->dsize) {
-               if (set->extensions & IPSET_EXT_DESTROY)
-                       mtype_ext_cleanup(set);
-               ip_set_free(map->extensions);
-       }
-       kfree(map);
+       if (set->dsize && set->extensions & IPSET_EXT_DESTROY)
+               mtype_ext_cleanup(set);
+       ip_set_free(map);
 
        set->data = NULL;
 }
@@ -92,16 +89,14 @@ mtype_head(struct ip_set *set, struct sk_buff *skb)
 {
        const struct mtype *map = set->data;
        struct nlattr *nested;
+       size_t memsize = sizeof(*map) + map->memsize;
 
        nested = ipset_nest_start(skb, IPSET_ATTR_DATA);
        if (!nested)
                goto nla_put_failure;
        if (mtype_do_head(skb, map) ||
            nla_put_net32(skb, IPSET_ATTR_REFERENCES, htonl(set->ref - 1)) ||
-           nla_put_net32(skb, IPSET_ATTR_MEMSIZE,
-                         htonl(sizeof(*map) +
-                               map->memsize +
-                               set->dsize * map->elements)))
+           nla_put_net32(skb, IPSET_ATTR_MEMSIZE, htonl(memsize)))
                goto nla_put_failure;
        if (unlikely(ip_set_put_flags(skb, set)))
                goto nla_put_failure;

diff --git a/net/netfilter/ipset/ip_set_bitmap_ip.c b/net/netfilter/ipset/ip_set_bitmap_ip.c
index 64a5643344180d813e3b3fc914c1b1c93dcd58be..4783efff0bde6b7c70707c8153563669308d995a 100644 (file)
--- a/net/netfilter/ipset/ip_set_bitmap_ip.c
+++ b/net/netfilter/ipset/ip_set_bitmap_ip.c
@@ -41,7 +41,6 @@ MODULE_ALIAS("ip_set_bitmap:ip");
 /* Type structure */
 struct bitmap_ip {
        void *members;          /* the set members */
-       void *extensions;       /* data extensions */
        u32 first_ip;           /* host byte order, included in range */
        u32 last_ip;            /* host byte order, included in range */
        u32 elements;           /* number of max elements in the set */
@@ -49,6 +48,8 @@ struct bitmap_ip {
        size_t memsize;         /* members size */
        u8 netmask;             /* subnet netmask */
        struct timer_list gc;   /* garbage collection */
+       unsigned char extensions[0]     /* data extensions */
+               __aligned(__alignof__(u64));
 };
 
 /* ADT structure for generic function args */
@@ -224,13 +225,6 @@ init_map_ip(struct ip_set *set, struct bitmap_ip *map,
        map->members = ip_set_alloc(map->memsize);
        if (!map->members)
                return false;
-       if (set->dsize) {
-               map->extensions = ip_set_alloc(set->dsize * elements);
-               if (!map->extensions) {
-                       kfree(map->members);
-                       return false;
-               }
-       }
        map->first_ip = first_ip;
        map->last_ip = last_ip;
        map->elements = elements;
@@ -316,13 +310,13 @@ bitmap_ip_create(struct net *net, struct ip_set *set, struct nlattr *tb[],
        pr_debug("hosts %u, elements %llu\n",
                 hosts, (unsigned long long)elements);
 
-       map = kzalloc(sizeof(*map), GFP_KERNEL);
+       set->dsize = ip_set_elem_len(set, tb, 0, 0);
+       map = ip_set_alloc(sizeof(*map) + elements * set->dsize);
        if (!map)
                return -ENOMEM;
 
        map->memsize = bitmap_bytes(0, elements - 1);
        set->variant = &bitmap_ip;
-       set->dsize = ip_set_elem_len(set, tb, 0);
        if (!init_map_ip(set, map, first_ip, last_ip,
                         elements, hosts, netmask)) {
                kfree(map);

diff --git a/net/netfilter/ipset/ip_set_bitmap_ipmac.c b/net/netfilter/ipset/ip_set_bitmap_ipmac.c
index 1430535118fb0d9c23c1f05c72e75034c1953cd7..29dde208381dac6cdfca67094dc4fc07bc85b40c 100644 (file)
--- a/net/netfilter/ipset/ip_set_bitmap_ipmac.c
+++ b/net/netfilter/ipset/ip_set_bitmap_ipmac.c
@@ -47,24 +47,26 @@ enum {
 /* Type structure */
 struct bitmap_ipmac {
        void *members;          /* the set members */
-       void *extensions;       /* MAC + data extensions */
        u32 first_ip;           /* host byte order, included in range */
        u32 last_ip;            /* host byte order, included in range */
        u32 elements;           /* number of max elements in the set */
        size_t memsize;         /* members size */
        struct timer_list gc;   /* garbage collector */
+       unsigned char extensions[0]     /* MAC + data extensions */
+               __aligned(__alignof__(u64));
 };
 
 /* ADT structure for generic function args */
 struct bitmap_ipmac_adt_elem {
+       unsigned char ether[ETH_ALEN] __aligned(2);
        u16 id;
-       unsigned char *ether;
+       u16 add_mac;
 };
 
 struct bitmap_ipmac_elem {
        unsigned char ether[ETH_ALEN];
        unsigned char filled;
-} __attribute__ ((aligned));
+} __aligned(__alignof__(u64));
 
 static inline u32
 ip_to_id(const struct bitmap_ipmac *m, u32 ip)
@@ -72,11 +74,11 @@ ip_to_id(const struct bitmap_ipmac *m, u32 ip)
        return ip - m->first_ip;
 }
 
-static inline struct bitmap_ipmac_elem *
-get_elem(void *extensions, u16 id, size_t dsize)
-{
-       return (struct bitmap_ipmac_elem *)(extensions + id * dsize);
-}
+#define get_elem(extensions, id, dsize)                \
+       (struct bitmap_ipmac_elem *)(extensions + (id) * (dsize))
+
+#define get_const_elem(extensions, id, dsize)  \
+       (const struct bitmap_ipmac_elem *)(extensions + (id) * (dsize))
 
 /* Common functions */
 
@@ -88,10 +90,9 @@ bitmap_ipmac_do_test(const struct bitmap_ipmac_adt_elem *e,
 
        if (!test_bit(e->id, map->members))
                return 0;
-       elem = get_elem(map->extensions, e->id, dsize);
-       if (elem->filled == MAC_FILLED)
-               return !e->ether ||
-                      ether_addr_equal(e->ether, elem->ether);
+       elem = get_const_elem(map->extensions, e->id, dsize);
+       if (e->add_mac && elem->filled == MAC_FILLED)
+               return ether_addr_equal(e->ether, elem->ether);
        /* Trigger kernel to fill out the ethernet address */
        return -EAGAIN;
 }
@@ -103,7 +104,7 @@ bitmap_ipmac_gc_test(u16 id, const struct bitmap_ipmac *map, size_t dsize)
 
        if (!test_bit(id, map->members))
                return 0;
-       elem = get_elem(map->extensions, id, dsize);
+       elem = get_const_elem(map->extensions, id, dsize);
        /* Timer not started for the incomplete elements */
        return elem->filled == MAC_FILLED;
 }
@@ -133,7 +134,7 @@ bitmap_ipmac_add_timeout(unsigned long *timeout,
                 * and we can reuse it later when MAC is filled out,
                 * possibly by the kernel
                 */
-               if (e->ether)
+               if (e->add_mac)
                        ip_set_timeout_set(timeout, t);
                else
                        *timeout = t;
@@ -150,7 +151,7 @@ bitmap_ipmac_do_add(const struct bitmap_ipmac_adt_elem *e,
        elem = get_elem(map->extensions, e->id, dsize);
        if (test_bit(e->id, map->members)) {
                if (elem->filled == MAC_FILLED) {
-                       if (e->ether &&
+                       if (e->add_mac &&
                            (flags & IPSET_FLAG_EXIST) &&
                            !ether_addr_equal(e->ether, elem->ether)) {
                                /* memcpy isn't atomic */
@@ -159,7 +160,7 @@ bitmap_ipmac_do_add(const struct bitmap_ipmac_adt_elem *e,
                                ether_addr_copy(elem->ether, e->ether);
                        }
                        return IPSET_ADD_FAILED;
-               } else if (!e->ether)
+               } else if (!e->add_mac)
                        /* Already added without ethernet address */
                        return IPSET_ADD_FAILED;
                /* Fill the MAC address and trigger the timer activation */
@@ -168,7 +169,7 @@ bitmap_ipmac_do_add(const struct bitmap_ipmac_adt_elem *e,
                ether_addr_copy(elem->ether, e->ether);
                elem->filled = MAC_FILLED;
                return IPSET_ADD_START_STORED_TIMEOUT;
-       } else if (e->ether) {
+       } else if (e->add_mac) {
                /* We can store MAC too */
                ether_addr_copy(elem->ether, e->ether);
                elem->filled = MAC_FILLED;
@@ -191,7 +192,7 @@ bitmap_ipmac_do_list(struct sk_buff *skb, const struct bitmap_ipmac *map,
                     u32 id, size_t dsize)
 {
        const struct bitmap_ipmac_elem *elem =
-               get_elem(map->extensions, id, dsize);
+               get_const_elem(map->extensions, id, dsize);
 
        return nla_put_ipaddr4(skb, IPSET_ATTR_IP,
                               htonl(map->first_ip + id)) ||
@@ -213,7 +214,7 @@ bitmap_ipmac_kadt(struct ip_set *set, const struct sk_buff *skb,
 {
        struct bitmap_ipmac *map = set->data;
        ipset_adtfn adtfn = set->variant->adt[adt];
-       struct bitmap_ipmac_adt_elem e = { .id = 0 };
+       struct bitmap_ipmac_adt_elem e = { .id = 0, .add_mac = 1 };
        struct ip_set_ext ext = IP_SET_INIT_KEXT(skb, opt, set);
        u32 ip;
 
@@ -231,7 +232,7 @@ bitmap_ipmac_kadt(struct ip_set *set, const struct sk_buff *skb,
                return -EINVAL;
 
        e.id = ip_to_id(map, ip);
-       e.ether = eth_hdr(skb)->h_source;
+       memcpy(e.ether, eth_hdr(skb)->h_source, ETH_ALEN);
 
        return adtfn(set, &e, &ext, &opt->ext, opt->cmdflags);
 }
@@ -265,11 +266,10 @@ bitmap_ipmac_uadt(struct ip_set *set, struct nlattr *tb[],
                return -IPSET_ERR_BITMAP_RANGE;
 
        e.id = ip_to_id(map, ip);
-       if (tb[IPSET_ATTR_ETHER])
-               e.ether = nla_data(tb[IPSET_ATTR_ETHER]);
-       else
-               e.ether = NULL;
-
+       if (tb[IPSET_ATTR_ETHER]) {
+               memcpy(e.ether, nla_data(tb[IPSET_ATTR_ETHER]), ETH_ALEN);
+               e.add_mac = 1;
+       }
        ret = adtfn(set, &e, &ext, &ext, flags);
 
        return ip_set_eexist(ret, flags) ? 0 : ret;
@@ -300,13 +300,6 @@ init_map_ipmac(struct ip_set *set, struct bitmap_ipmac *map,
        map->members = ip_set_alloc(map->memsize);
        if (!map->members)
                return false;
-       if (set->dsize) {
-               map->extensions = ip_set_alloc(set->dsize * elements);
-               if (!map->extensions) {
-                       kfree(map->members);
-                       return false;
-               }
-       }
        map->first_ip = first_ip;
        map->last_ip = last_ip;
        map->elements = elements;
@@ -361,14 +354,15 @@ bitmap_ipmac_create(struct net *net, struct ip_set *set, struct nlattr *tb[],
        if (elements > IPSET_BITMAP_MAX_RANGE + 1)
                return -IPSET_ERR_BITMAP_RANGE_SIZE;
 
-       map = kzalloc(sizeof(*map), GFP_KERNEL);
+       set->dsize = ip_set_elem_len(set, tb,
+                                    sizeof(struct bitmap_ipmac_elem),
+                                    __alignof__(struct bitmap_ipmac_elem));
+       map = ip_set_alloc(sizeof(*map) + elements * set->dsize);
        if (!map)
                return -ENOMEM;
 
        map->memsize = bitmap_bytes(0, elements - 1);
        set->variant = &bitmap_ipmac;
-       set->dsize = ip_set_elem_len(set, tb,
-                                    sizeof(struct bitmap_ipmac_elem));
        if (!init_map_ipmac(set, map, first_ip, last_ip, elements)) {
                kfree(map);
                return -ENOMEM;

diff --git a/net/netfilter/ipset/ip_set_bitmap_port.c b/net/netfilter/ipset/ip_set_bitmap_port.c
index 5338ccd5da461eea08142479fd8315a208fc50c4..7f0c733358a428225e15afe611aad1edcd52812a 100644 (file)
--- a/net/netfilter/ipset/ip_set_bitmap_port.c
+++ b/net/netfilter/ipset/ip_set_bitmap_port.c
@@ -35,12 +35,13 @@ MODULE_ALIAS("ip_set_bitmap:port");
 /* Type structure */
 struct bitmap_port {
        void *members;          /* the set members */
-       void *extensions;       /* data extensions */
        u16 first_port;         /* host byte order, included in range */
        u16 last_port;          /* host byte order, included in range */
        u32 elements;           /* number of max elements in the set */
        size_t memsize;         /* members size */
        struct timer_list gc;   /* garbage collection */
+       unsigned char extensions[0]     /* data extensions */
+               __aligned(__alignof__(u64));
 };
 
 /* ADT structure for generic function args */
@@ -209,13 +210,6 @@ init_map_port(struct ip_set *set, struct bitmap_port *map,
        map->members = ip_set_alloc(map->memsize);
        if (!map->members)
                return false;
-       if (set->dsize) {
-               map->extensions = ip_set_alloc(set->dsize * map->elements);
-               if (!map->extensions) {
-                       kfree(map->members);
-                       return false;
-               }
-       }
        map->first_port = first_port;
        map->last_port = last_port;
        set->timeout = IPSET_NO_TIMEOUT;
@@ -232,6 +226,7 @@ bitmap_port_create(struct net *net, struct ip_set *set, struct nlattr *tb[],
 {
        struct bitmap_port *map;
        u16 first_port, last_port;
+       u32 elements;
 
        if (unlikely(!ip_set_attr_netorder(tb, IPSET_ATTR_PORT) ||
                     !ip_set_attr_netorder(tb, IPSET_ATTR_PORT_TO) ||
@@ -248,14 +243,15 @@ bitmap_port_create(struct net *net, struct ip_set *set, struct nlattr *tb[],
                last_port = tmp;
        }
 
-       map = kzalloc(sizeof(*map), GFP_KERNEL);
+       elements = last_port - first_port + 1;
+       set->dsize = ip_set_elem_len(set, tb, 0, 0);
+       map = ip_set_alloc(sizeof(*map) + elements * set->dsize);
        if (!map)
                return -ENOMEM;
 
-       map->elements = last_port - first_port + 1;
+       map->elements = elements;
        map->memsize = bitmap_bytes(0, map->elements);
        set->variant = &bitmap_port;
-       set->dsize = ip_set_elem_len(set, tb, 0);
        if (!init_map_port(set, map, first_port, last_port)) {
                kfree(map);
                return -ENOMEM;

diff --git a/net/netfilter/ipset/ip_set_core.c b/net/netfilter/ipset/ip_set_core.c
index 69ab9c2634e1566fac04e9509cf4c51e64ea36e8..54f3d7cb23e6efe4a3d9c4084df66cca76a44b03 100644 (file)
--- a/net/netfilter/ipset/ip_set_core.c
+++ b/net/netfilter/ipset/ip_set_core.c
@@ -364,25 +364,27 @@ add_extension(enum ip_set_ext_id id, u32 flags, struct nlattr *tb[])
 }
 
 size_t
-ip_set_elem_len(struct ip_set *set, struct nlattr *tb[], size_t len)
+ip_set_elem_len(struct ip_set *set, struct nlattr *tb[], size_t len,
+               size_t align)
 {
        enum ip_set_ext_id id;
-       size_t offset = len;
        u32 cadt_flags = 0;
 
        if (tb[IPSET_ATTR_CADT_FLAGS])
                cadt_flags = ip_set_get_h32(tb[IPSET_ATTR_CADT_FLAGS]);
        if (cadt_flags & IPSET_FLAG_WITH_FORCEADD)
                set->flags |= IPSET_CREATE_FLAG_FORCEADD;
+       if (!align)
+               align = 1;
        for (id = 0; id < IPSET_EXT_ID_MAX; id++) {
                if (!add_extension(id, cadt_flags, tb))
                        continue;
-               offset = ALIGN(offset, ip_set_extensions[id].align);
-               set->offset[id] = offset;
+               len = ALIGN(len, ip_set_extensions[id].align);
+               set->offset[id] = len;
                set->extensions |= ip_set_extensions[id].type;
-               offset += ip_set_extensions[id].len;
+               len += ip_set_extensions[id].len;
        }
-       return offset;
+       return ALIGN(len, align);
 }
 EXPORT_SYMBOL_GPL(ip_set_elem_len);
 

diff --git a/net/netfilter/ipset/ip_set_hash_gen.h b/net/netfilter/ipset/ip_set_hash_gen.h
index 691b54fcaf2a47189665454fdcaff419236b020f..4ff22194ce5572ef32a06062c348ff8022676699 100644 (file)
--- a/net/netfilter/ipset/ip_set_hash_gen.h
+++ b/net/netfilter/ipset/ip_set_hash_gen.h
@@ -72,8 +72,9 @@ struct hbucket {
        DECLARE_BITMAP(used, AHASH_MAX_TUNED);
        u8 size;                /* size of the array */
        u8 pos;                 /* position of the first free entry */
-       unsigned char value[0]; /* the array of the values */
-} __attribute__ ((aligned));
+       unsigned char value[0]  /* the array of the values */
+               __aligned(__alignof__(u64));
+};
 
 /* The hash table: the table size stored here in order to make resizing easy */
 struct htable {
@@ -1323,12 +1324,14 @@ IPSET_TOKEN(HTYPE, _create)(struct net *net, struct ip_set *set,
 #endif
                set->variant = &IPSET_TOKEN(HTYPE, 4_variant);
                set->dsize = ip_set_elem_len(set, tb,
-                               sizeof(struct IPSET_TOKEN(HTYPE, 4_elem)));
+                       sizeof(struct IPSET_TOKEN(HTYPE, 4_elem)),
+                       __alignof__(struct IPSET_TOKEN(HTYPE, 4_elem)));
 #ifndef IP_SET_PROTO_UNDEF
        } else {
                set->variant = &IPSET_TOKEN(HTYPE, 6_variant);
                set->dsize = ip_set_elem_len(set, tb,
-                               sizeof(struct IPSET_TOKEN(HTYPE, 6_elem)));
+                       sizeof(struct IPSET_TOKEN(HTYPE, 6_elem)),
+                       __alignof__(struct IPSET_TOKEN(HTYPE, 6_elem)));
        }
 #endif
        if (tb[IPSET_ATTR_TIMEOUT]) {

diff --git a/net/netfilter/ipset/ip_set_list_set.c b/net/netfilter/ipset/ip_set_list_set.c
index 5a30ce6e8c90d278ac37cb0115f45a4390f9a6d7..bbede95c9f68cee1a1e0b5c285231c78d987df63 100644 (file)
--- a/net/netfilter/ipset/ip_set_list_set.c
+++ b/net/netfilter/ipset/ip_set_list_set.c
@@ -31,7 +31,7 @@ struct set_elem {
        struct rcu_head rcu;
        struct list_head list;
        ip_set_id_t id;
-};
+} __aligned(__alignof__(u64));
 
 struct set_adt_elem {
        ip_set_id_t id;
@@ -618,7 +618,8 @@ list_set_create(struct net *net, struct ip_set *set, struct nlattr *tb[],
                size = IP_SET_LIST_MIN_SIZE;
 
        set->variant = &set_variant;
-       set->dsize = ip_set_elem_len(set, tb, sizeof(struct set_elem));
+       set->dsize = ip_set_elem_len(set, tb, sizeof(struct set_elem),
+                                    __alignof__(struct set_elem));
        if (!init_list_set(net, set, size))
                return -ENOMEM;
        if (tb[IPSET_ATTR_TIMEOUT]) {