* [PATCH] netfilter: nf_tables: fix loop checking with set element data
@ 2014-02-07 15:23 Pablo Neira Ayuso
2014-02-07 15:47 ` Patrick McHardy
0 siblings, 1 reply; 4+ messages in thread
From: Pablo Neira Ayuso @ 2014-02-07 15:23 UTC (permalink / raw)
To: netfilter-devel; +Cc: kaber, --confirm=always
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(+)
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
^ permalink raw reply related [flat|nested] 4+ messages in thread* Re: [PATCH] netfilter: nf_tables: fix loop checking with set element data
2014-02-07 15:23 [PATCH] netfilter: nf_tables: fix loop checking with set element data Pablo Neira Ayuso
@ 2014-02-07 15:47 ` Patrick McHardy
2014-02-07 15:51 ` Patrick McHardy
0 siblings, 1 reply; 4+ messages in thread
From: Patrick McHardy @ 2014-02-07 15:47 UTC (permalink / raw)
To: Pablo Neira Ayuso; +Cc: netfilter-devel
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
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH] netfilter: nf_tables: fix loop checking with set element data
2014-02-07 15:47 ` Patrick McHardy
@ 2014-02-07 15:51 ` Patrick McHardy
2014-02-07 16:20 ` Pablo Neira Ayuso
0 siblings, 1 reply; 4+ messages in thread
From: Patrick McHardy @ 2014-02-07 15:51 UTC (permalink / raw)
To: Pablo Neira Ayuso; +Cc: netfilter-devel
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
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH] netfilter: nf_tables: fix loop checking with set element data
2014-02-07 15:51 ` Patrick McHardy
@ 2014-02-07 16:20 ` Pablo Neira Ayuso
0 siblings, 0 replies; 4+ messages in thread
From: Pablo Neira Ayuso @ 2014-02-07 16:20 UTC (permalink / raw)
To: Patrick McHardy; +Cc: netfilter-devel
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().
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2014-02-07 16:20 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-02-07 15:23 [PATCH] netfilter: nf_tables: fix loop checking with set element data Pablo Neira Ayuso
2014-02-07 15:47 ` Patrick McHardy
2014-02-07 15:51 ` Patrick McHardy
2014-02-07 16:20 ` Pablo Neira Ayuso
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).