Alignment problem between 64bit kernel 32bit userspace

Sven-Haegar Koch reported the issue:

sims:~# iptables -A OUTPUT -m set --match-set testset src -j ACCEPT
iptables: Invalid argument. Run `dmesg' for more information.

In syslog:
x_tables: ip_tables: set.3 match: invalid size 48 (kernel) != (user) 32

which was introduced by the counter extension in ipset.

The patch fixes the alignment issue with introducing a new set match
revision with the fixed underlying 'struct ip_set_counter_match'
structure.

Signed-off-by: Jozsef Kadlecsik <kadlec@blackhole.kfki.hu>

diff --git a/extensions/libxt_set.c b/extensions/libxt_set.c
index 2cb9e78a85f97e30f36d7855b46efbc1de43a57d..679c04c75b0f65da8f6e9a5665c22644d3d6c58b 100644 (file)
--- a/extensions/libxt_set.c
+++ b/extensions/libxt_set.c
@@ -52,7 +52,7 @@ static int
 set_parse_v0(int c, char **argv, int invert, unsigned int *flags,
             const void *entry, struct xt_entry_match **match)
 {
-       struct xt_set_info_match_v0 *myinfo = 
+       struct xt_set_info_match_v0 *myinfo =
                (struct xt_set_info_match_v0 *) (*match)->data;
        struct xt_set_info_v0 *info = &myinfo->match_set;
 
@@ -82,7 +82,7 @@ set_parse_v0(int c, char **argv, int invert, unsigned int *flags,
                parse_dirs_v0(argv[optind], info);
                DEBUGP("parse: set index %u\n", info->index);
                optind++;
-               
+
                *flags = 1;
                break;
        }
@@ -132,7 +132,7 @@ static int
 set_parse_v1(int c, char **argv, int invert, unsigned int *flags,
             const void *entry, struct xt_entry_match **match)
 {
-       struct xt_set_info_match_v1 *myinfo = 
+       struct xt_set_info_match_v1 *myinfo =
                (struct xt_set_info_match_v1 *) (*match)->data;
        struct xt_set_info *info = &myinfo->match_set;
 
@@ -162,7 +162,7 @@ set_parse_v1(int c, char **argv, int invert, unsigned int *flags,
                parse_dirs(argv[optind], info);
                DEBUGP("parse: set index %u\n", info->index);
                optind++;
-               
+
                *flags = 1;
                break;
        }
@@ -227,7 +227,7 @@ static int
 set_parse_v2(int c, char **argv, int invert, unsigned int *flags,
             const void *entry, struct xt_entry_match **match)
 {
-       struct xt_set_info_match_v1 *myinfo = 
+       struct xt_set_info_match_v1 *myinfo =
                (struct xt_set_info_match_v1 *) (*match)->data;
        struct xt_set_info *info = &myinfo->match_set;
 
@@ -260,7 +260,7 @@ set_parse_v2(int c, char **argv, int invert, unsigned int *flags,
                parse_dirs(argv[optind], info);
                DEBUGP("parse: set index %u\n", info->index);
                optind++;
-               
+
                *flags = 1;
                break;
        }
@@ -334,7 +334,7 @@ static int
 set_parse_v3(int c, char **argv, int invert, unsigned int *flags,
             const void *entry, struct xt_entry_match **match)
 {
-       struct xt_set_info_match_v3 *info = 
+       struct xt_set_info_match_v3 *info =
                (struct xt_set_info_match_v3 *) (*match)->data;
 
        switch (c) {
@@ -437,7 +437,7 @@ set_parse_v3(int c, char **argv, int invert, unsigned int *flags,
                parse_dirs(argv[optind], &info->match_set);
                DEBUGP("parse: set index %u\n", info->match_set.index);
                optind++;
-               
+
                *flags = 1;
                break;
        }
@@ -446,7 +446,7 @@ set_parse_v3(int c, char **argv, int invert, unsigned int *flags,
 }
 
 static void
-set_printv3_counter(const struct ip_set_counter_match *c, const char *name,
+set_printv3_counter(const struct ip_set_counter_match0 *c, const char *name,
                    const char *sep)
 {
        switch (c->op) {
@@ -497,6 +497,174 @@ set_save_v3(const void *ip, const struct xt_entry_match *match)
        set_print_v3_matchinfo(info, "--match-set", "--");
 }
 
+/* Revision 4 */
+static int
+set_parse_v4(int c, char **argv, int invert, unsigned int *flags,
+            const void *entry, struct xt_entry_match **match)
+{
+       struct xt_set_info_match_v4 *info =
+               (struct xt_set_info_match_v4 *) (*match)->data;
+
+       switch (c) {
+       case 'a':
+               if (invert)
+                       info->flags |= IPSET_FLAG_SKIP_SUBCOUNTER_UPDATE;
+               break;
+       case '0':
+               if (info->bytes.op != IPSET_COUNTER_NONE)
+                       xtables_error(PARAMETER_PROBLEM,
+                                     "only one of the --bytes-[eq|lt|gt]"
+                                     " is allowed\n");
+               if (invert)
+                       xtables_error(PARAMETER_PROBLEM,
+                                     "--bytes-gt option cannot be inverted\n");
+               info->bytes.op = IPSET_COUNTER_GT;
+               info->bytes.value = parse_counter(optarg);
+               break;
+       case '9':
+               if (info->bytes.op != IPSET_COUNTER_NONE)
+                       xtables_error(PARAMETER_PROBLEM,
+                                     "only one of the --bytes-[eq|lt|gt]"
+                                     " is allowed\n");
+               if (invert)
+                       xtables_error(PARAMETER_PROBLEM,
+                                     "--bytes-lt option cannot be inverted\n");
+               info->bytes.op = IPSET_COUNTER_LT;
+               info->bytes.value = parse_counter(optarg);
+               break;
+       case '8':
+               if (info->bytes.op != IPSET_COUNTER_NONE)
+                       xtables_error(PARAMETER_PROBLEM,
+                                     "only one of the --bytes-[eq|lt|gt]"
+                                     " is allowed\n");
+               info->bytes.op = invert ? IPSET_COUNTER_NE : IPSET_COUNTER_EQ;
+               info->bytes.value = parse_counter(optarg);
+               break;
+       case '7':
+               if (info->packets.op != IPSET_COUNTER_NONE)
+                       xtables_error(PARAMETER_PROBLEM,
+                                     "only one of the --packets-[eq|lt|gt]"
+                                     " is allowed\n");
+               if (invert)
+                       xtables_error(PARAMETER_PROBLEM,
+                                     "--packets-gt option cannot be inverted\n");
+               info->packets.op = IPSET_COUNTER_GT;
+               info->packets.value = parse_counter(optarg);
+               break;
+       case '6':
+               if (info->packets.op != IPSET_COUNTER_NONE)
+                       xtables_error(PARAMETER_PROBLEM,
+                                     "only one of the --packets-[eq|lt|gt]"
+                                     " is allowed\n");
+               if (invert)
+                       xtables_error(PARAMETER_PROBLEM,
+                                     "--packets-lt option cannot be inverted\n");
+               info->packets.op = IPSET_COUNTER_LT;
+               info->packets.value = parse_counter(optarg);
+               break;
+       case '5':
+               if (info->packets.op != IPSET_COUNTER_NONE)
+                       xtables_error(PARAMETER_PROBLEM,
+                                     "only one of the --packets-[eq|lt|gt]"
+                                     " is allowed\n");
+               info->packets.op = invert ? IPSET_COUNTER_NE : IPSET_COUNTER_EQ;
+               info->packets.value = parse_counter(optarg);
+               break;
+       case '4':
+               if (invert)
+                       info->flags |= IPSET_FLAG_SKIP_COUNTER_UPDATE;
+               break;
+       case '3':
+               if (invert)
+                       xtables_error(PARAMETER_PROBLEM,
+                                     "--return-nomatch flag cannot be inverted\n");
+               info->flags |= IPSET_FLAG_RETURN_NOMATCH;
+               break;
+       case '2':
+               fprintf(stderr,
+                       "--set option deprecated, please use --match-set\n");
+       case '1':               /* --match-set <set> <flag>[,<flag> */
+               if (info->match_set.dim)
+                       xtables_error(PARAMETER_PROBLEM,
+                                     "--match-set can be specified only once");
+               if (invert)
+                       info->match_set.flags |= IPSET_INV_MATCH;
+
+               if (!argv[optind]
+                   || argv[optind][0] == '-'
+                   || argv[optind][0] == '!')
+                       xtables_error(PARAMETER_PROBLEM,
+                                     "--match-set requires two args.");
+
+               if (strlen(optarg) > IPSET_MAXNAMELEN - 1)
+                       xtables_error(PARAMETER_PROBLEM,
+                                     "setname `%s' too long, max %d characters.",
+                                     optarg, IPSET_MAXNAMELEN - 1);
+
+               get_set_byname(optarg, &info->match_set);
+               parse_dirs(argv[optind], &info->match_set);
+               DEBUGP("parse: set index %u\n", info->match_set.index);
+               optind++;
+
+               *flags = 1;
+               break;
+       }
+
+       return 1;
+}
+
+static void
+set_printv4_counter(const struct ip_set_counter_match *c, const char *name,
+                   const char *sep)
+{
+       switch (c->op) {
+       case IPSET_COUNTER_EQ:
+               printf(" %s%s-eq %llu", sep, name, c->value);
+               break;
+       case IPSET_COUNTER_NE:
+               printf(" ! %s%s-eq %llu", sep, name, c->value);
+               break;
+       case IPSET_COUNTER_LT:
+               printf(" %s%s-lt %llu", sep, name, c->value);
+               break;
+       case IPSET_COUNTER_GT:
+               printf(" %s%s-gt %llu", sep, name, c->value);
+               break;
+       }
+}
+
+static void
+set_print_v4_matchinfo(const struct xt_set_info_match_v4 *info,
+                      const char *opt, const char *sep)
+{
+       print_match(opt, &info->match_set);
+       if (info->flags & IPSET_FLAG_RETURN_NOMATCH)
+               printf(" %sreturn-nomatch", sep);
+       if ((info->flags & IPSET_FLAG_SKIP_COUNTER_UPDATE))
+               printf(" ! %supdate-counters", sep);
+       if ((info->flags & IPSET_FLAG_SKIP_SUBCOUNTER_UPDATE))
+               printf(" ! %supdate-subcounters", sep);
+       set_printv4_counter(&info->packets, "packets", sep);
+       set_printv4_counter(&info->bytes, "bytes", sep);
+}
+
+/* Prints out the matchinfo. */
+static void
+set_print_v4(const void *ip, const struct xt_entry_match *match, int numeric)
+{
+       const struct xt_set_info_match_v4 *info = (const void *)match->data;
+
+       set_print_v4_matchinfo(info, "match-set", "");
+}
+
+static void
+set_save_v4(const void *ip, const struct xt_entry_match *match)
+{
+       const struct xt_set_info_match_v4 *info = (const void *)match->data;
+
+       set_print_v4_matchinfo(info, "--match-set", "--");
+}
+
 static struct xtables_match set_mt_reg[] = {
        {
                .name           = "set",
@@ -554,6 +722,20 @@ static struct xtables_match set_mt_reg[] = {
                .save           = set_save_v3,
                .extra_opts     = set_opts_v3,
        },
+       {
+               .name           = "set",
+               .revision       = 4,
+               .version        = XTABLES_VERSION,
+               .family         = NFPROTO_UNSPEC,
+               .size           = XT_ALIGN(sizeof(struct xt_set_info_match_v4)),
+               .userspacesize  = XT_ALIGN(sizeof(struct xt_set_info_match_v4)),
+               .help           = set_help_v3,
+               .parse          = set_parse_v4,
+               .final_check    = set_check_v0,
+               .print          = set_print_v4,
+               .save           = set_save_v4,
+               .extra_opts     = set_opts_v3,
+       },
 };
 
 void _init(void)

diff --git a/include/linux/netfilter/ipset/ip_set.h b/include/linux/netfilter/ipset/ip_set.h
index 7f1d604109a35d5036221a02db6da92af0e91fab..b69bbc7294cce1793ffc846aa35a8998c46e76a0 100644 (file)
--- a/include/linux/netfilter/ipset/ip_set.h
+++ b/include/linux/netfilter/ipset/ip_set.h
@@ -236,11 +236,17 @@ enum {
        IPSET_COUNTER_GT,
 };
 
-struct ip_set_counter_match {
+/* Backward compatibility for set match v3 */
+struct ip_set_counter_match0 {
        __u8 op;
        __u64 value;
 };
 
+struct ip_set_counter_match {
+       __aligned_u64 value;
+       __u8 op;
+};
+
 /* Interface to iptables/ip6tables */
 
 #define SO_IP_SET              83

diff --git a/include/linux/netfilter/xt_set.h b/include/linux/netfilter/xt_set.h
index d6a1df1f2947ba6aa0cc8680a2efb8f61a0ae178..4210c9bff886cebb30403e7c2ee107f4d2fa6a0a 100644 (file)
--- a/include/linux/netfilter/xt_set.h
+++ b/include/linux/netfilter/xt_set.h
@@ -65,6 +65,15 @@ struct xt_set_info_target_v2 {
 /* Revision 3 match */
 
 struct xt_set_info_match_v3 {
+       struct xt_set_info match_set;
+       struct ip_set_counter_match0 packets;
+       struct ip_set_counter_match0 bytes;
+       __u32 flags;
+};
+
+/* Revision 4 match */
+
+struct xt_set_info_match_v4 {
        struct xt_set_info match_set;
        struct ip_set_counter_match packets;
        struct ip_set_counter_match bytes;