Message ID | 1391786631-4070-1-git-send-email-pablo@netfilter.org |
---|---|
State | Superseded |
Headers | show |
On Fri, Feb 07, 2014 at 04:23:51PM +0100, Pablo Neira Ayuso wrote: > This patch fixes two bugs: > > * Restrict the validation to verdict maps only, since struct nft_data > is declared as a union, data[0] may contain the values -3 or -4 which > match with NFT_JUMP and NFT_GOTO. I think this will result in crash > while performing the chain loop validation. > > * Access to uninitialized data for end interval elements. The element > data part is uninitialized in interval end elements. > That's not necessary. The function is only invoked for NFT_DATA_VERDICT. > Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org> > --- > net/netfilter/nf_tables_api.c | 5 +++++ > 1 file changed, 5 insertions(+) > > diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c > index d0c790e3e..522becb 100644 > --- a/net/netfilter/nf_tables_api.c > +++ b/net/netfilter/nf_tables_api.c > @@ -2998,6 +2998,11 @@ static int nf_tables_loop_check_setelem(const struct nft_ctx *ctx, > const struct nft_set_iter *iter, > const struct nft_set_elem *elem) > { > + if (!(set->flags & NFT_SET_MAP) || > + set->dtype != NFT_DATA_VERDICT || > + elem->flags & NFT_SET_ELEM_INTERVAL_END) > + return 0; > + > switch (elem->data.verdict) { > case NFT_JUMP: > case NFT_GOTO: > -- > 1.7.10.4 -- To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, Feb 07, 2014 at 03:47:34PM +0000, Patrick McHardy wrote: > On Fri, Feb 07, 2014 at 04:23:51PM +0100, Pablo Neira Ayuso wrote: > > This patch fixes two bugs: > > > > * Restrict the validation to verdict maps only, since struct nft_data > > is declared as a union, data[0] may contain the values -3 or -4 which > > match with NFT_JUMP and NFT_GOTO. I think this will result in crash > > while performing the chain loop validation. > > > > * Access to uninitialized data for end interval elements. The element > > data part is uninitialized in interval end elements. > > > > That's not necessary. The function is only invoked for NFT_DATA_VERDICT. > > > > Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org> > > --- > > net/netfilter/nf_tables_api.c | 5 +++++ > > 1 file changed, 5 insertions(+) > > > > diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c > > index d0c790e3e..522becb 100644 > > --- a/net/netfilter/nf_tables_api.c > > +++ b/net/netfilter/nf_tables_api.c > > @@ -2998,6 +2998,11 @@ static int nf_tables_loop_check_setelem(const struct nft_ctx *ctx, > > const struct nft_set_iter *iter, > > const struct nft_set_elem *elem) > > { > > + if (!(set->flags & NFT_SET_MAP) || > > + set->dtype != NFT_DATA_VERDICT || > > + elem->flags & NFT_SET_ELEM_INTERVAL_END) > > + return 0; > > + Right, I get the intention now. This function is only called for maps with dtype verdict, so just the check for SET_ELEM_INTERVAL_END is all we need. > > switch (elem->data.verdict) { > > case NFT_JUMP: > > case NFT_GOTO: > > -- > > 1.7.10.4 > -- > To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, Feb 07, 2014 at 03:51:05PM +0000, Patrick McHardy wrote: > On Fri, Feb 07, 2014 at 03:47:34PM +0000, Patrick McHardy wrote: > > On Fri, Feb 07, 2014 at 04:23:51PM +0100, Pablo Neira Ayuso wrote: > > > This patch fixes two bugs: > > > > > > * Restrict the validation to verdict maps only, since struct nft_data > > > is declared as a union, data[0] may contain the values -3 or -4 which > > > match with NFT_JUMP and NFT_GOTO. I think this will result in crash > > > while performing the chain loop validation. > > > > > > * Access to uninitialized data for end interval elements. The element > > > data part is uninitialized in interval end elements. > > > > > > > That's not necessary. The function is only invoked for NFT_DATA_VERDICT. > > > > > > > Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org> > > > --- > > > net/netfilter/nf_tables_api.c | 5 +++++ > > > 1 file changed, 5 insertions(+) > > > > > > diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c > > > index d0c790e3e..522becb 100644 > > > --- a/net/netfilter/nf_tables_api.c > > > +++ b/net/netfilter/nf_tables_api.c > > > @@ -2998,6 +2998,11 @@ static int nf_tables_loop_check_setelem(const struct nft_ctx *ctx, > > > const struct nft_set_iter *iter, > > > const struct nft_set_elem *elem) > > > { > > > + if (!(set->flags & NFT_SET_MAP) || > > > + set->dtype != NFT_DATA_VERDICT || > > > + elem->flags & NFT_SET_ELEM_INTERVAL_END) > > > + return 0; > > > + > > Right, I get the intention now. > > This function is only called for maps with dtype verdict, so just the check > for SET_ELEM_INTERVAL_END is all we need. Right, I overlook this: list_for_each_entry(set, &ctx->table->sets, list) { if (!(set->flags & NFT_SET_MAP) || set->dtype != NFT_DATA_VERDICT) continue; at the top of nf_tables_check_loops(). -- To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c index d0c790e3e..522becb 100644 --- a/net/netfilter/nf_tables_api.c +++ b/net/netfilter/nf_tables_api.c @@ -2998,6 +2998,11 @@ static int nf_tables_loop_check_setelem(const struct nft_ctx *ctx, const struct nft_set_iter *iter, const struct nft_set_elem *elem) { + if (!(set->flags & NFT_SET_MAP) || + set->dtype != NFT_DATA_VERDICT || + elem->flags & NFT_SET_ELEM_INTERVAL_END) + return 0; + switch (elem->data.verdict) { case NFT_JUMP: case NFT_GOTO:
This patch fixes two bugs: * Restrict the validation to verdict maps only, since struct nft_data is declared as a union, data[0] may contain the values -3 or -4 which match with NFT_JUMP and NFT_GOTO. I think this will result in crash while performing the chain loop validation. * Access to uninitialized data for end interval elements. The element data part is uninitialized in interval end elements. Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org> --- net/netfilter/nf_tables_api.c | 5 +++++ 1 file changed, 5 insertions(+)