From a40cd9b784590ee09f1be4897f28bb0b2ce1096d Mon Sep 17 00:00:00 2001 From: Jozsef Kadlecsik Date: Thu, 6 Nov 2014 19:15:26 +0100 Subject: 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 --- extensions/libxt_set.c | 200 +++++++++++++++++++++++++++++++-- include/linux/netfilter/ipset/ip_set.h | 8 +- include/linux/netfilter/xt_set.h | 9 ++ 3 files changed, 207 insertions(+), 10 deletions(-) diff --git a/extensions/libxt_set.c b/extensions/libxt_set.c index 2cb9e78a..679c04c7 100644 --- 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 [, */ + 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 7f1d6041..b69bbc72 100644 --- 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 d6a1df1f..4210c9bf 100644 --- 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; -- cgit v1.2.3