* [PATCH nf-next v2] netfilter: nf_tables: fix possible null-pointer dereference in object update
@ 2019-09-04 12:29 Fernando Fernandez Mancera
2019-09-05 11:22 ` Pablo Neira Ayuso
2019-11-01 14:42 ` Eric Garver
0 siblings, 2 replies; 6+ messages in thread
From: Fernando Fernandez Mancera @ 2019-09-04 12:29 UTC (permalink / raw)
To: netfilter-devel; +Cc: Fernando Fernandez Mancera
Not all objects need an update operation. If the object type doesn't implement
an update operation and the user tries to update it there will be a EOPNOTSUPP
error instead of a null pointer.
Fixes: d62d0ba97b58 ("netfilter: nf_tables: Introduce stateful object update operation")
Signed-off-by: Fernando Fernandez Mancera <ffmancera@riseup.net>
---
net/netfilter/nf_tables_api.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c
index cf767bc58e18..013d28899cab 100644
--- a/net/netfilter/nf_tables_api.c
+++ b/net/netfilter/nf_tables_api.c
@@ -5140,6 +5140,9 @@ static int nf_tables_updobj(const struct nft_ctx *ctx,
struct nft_trans *trans;
int err;
+ if (!obj->ops->update)
+ return -EOPNOTSUPP;
+
trans = nft_trans_alloc(ctx, NFT_MSG_NEWOBJ,
sizeof(struct nft_trans_obj));
if (!trans)
--
2.20.1
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH nf-next v2] netfilter: nf_tables: fix possible null-pointer dereference in object update 2019-09-04 12:29 [PATCH nf-next v2] netfilter: nf_tables: fix possible null-pointer dereference in object update Fernando Fernandez Mancera @ 2019-09-05 11:22 ` Pablo Neira Ayuso 2019-11-01 14:42 ` Eric Garver 1 sibling, 0 replies; 6+ messages in thread From: Pablo Neira Ayuso @ 2019-09-05 11:22 UTC (permalink / raw) To: Fernando Fernandez Mancera; +Cc: netfilter-devel On Wed, Sep 04, 2019 at 02:29:07PM +0200, Fernando Fernandez Mancera wrote: > Not all objects need an update operation. If the object type doesn't implement > an update operation and the user tries to update it there will be a EOPNOTSUPP > error instead of a null pointer. Applied, thanks. ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH nf-next v2] netfilter: nf_tables: fix possible null-pointer dereference in object update 2019-09-04 12:29 [PATCH nf-next v2] netfilter: nf_tables: fix possible null-pointer dereference in object update Fernando Fernandez Mancera 2019-09-05 11:22 ` Pablo Neira Ayuso @ 2019-11-01 14:42 ` Eric Garver 2019-11-01 15:01 ` Fernando Fernández Mancera 1 sibling, 1 reply; 6+ messages in thread From: Eric Garver @ 2019-11-01 14:42 UTC (permalink / raw) To: Fernando Fernandez Mancera; +Cc: netfilter-devel Hi Fernando, On Wed, Sep 04, 2019 at 02:29:07PM +0200, Fernando Fernandez Mancera wrote: > Not all objects need an update operation. If the object type doesn't implement > an update operation and the user tries to update it there will be a EOPNOTSUPP > error instead of a null pointer. > > Fixes: d62d0ba97b58 ("netfilter: nf_tables: Introduce stateful object update operation") > Signed-off-by: Fernando Fernandez Mancera <ffmancera@riseup.net> > --- > net/netfilter/nf_tables_api.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c > index cf767bc58e18..013d28899cab 100644 > --- a/net/netfilter/nf_tables_api.c > +++ b/net/netfilter/nf_tables_api.c > @@ -5140,6 +5140,9 @@ static int nf_tables_updobj(const struct nft_ctx *ctx, > struct nft_trans *trans; > int err; > > + if (!obj->ops->update) > + return -EOPNOTSUPP; > + > trans = nft_trans_alloc(ctx, NFT_MSG_NEWOBJ, > sizeof(struct nft_trans_obj)); > if (!trans) > -- > 2.20.1 I think this introduced a regression when adding an object that already exists: # nft add table inet foobar # nft add counter inet foobar my_counter # nft add counter inet foobar my_counter Error: Could not process rule: Operation not supported add counter inet foobar my_counter ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ It applies to all objects that don't provide an update handler; counter, ct helper, ct timeout, ct exception, etc. ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH nf-next v2] netfilter: nf_tables: fix possible null-pointer dereference in object update 2019-11-01 14:42 ` Eric Garver @ 2019-11-01 15:01 ` Fernando Fernández Mancera 2019-11-01 15:11 ` Eric Garver 0 siblings, 1 reply; 6+ messages in thread From: Fernando Fernández Mancera @ 2019-11-01 15:01 UTC (permalink / raw) To: Eric Garver; +Cc: netfilter-devel El 1 de noviembre de 2019 15:42:46 CET, Eric Garver <eric@garver.life> escribió: >Hi Fernando, > >On Wed, Sep 04, 2019 at 02:29:07PM +0200, Fernando Fernandez Mancera >wrote: >> Not all objects need an update operation. If the object type doesn't >implement >> an update operation and the user tries to update it there will be a >EOPNOTSUPP >> error instead of a null pointer. >> >> Fixes: d62d0ba97b58 ("netfilter: nf_tables: Introduce stateful object >update operation") >> Signed-off-by: Fernando Fernandez Mancera <ffmancera@riseup.net> >> --- >> net/netfilter/nf_tables_api.c | 3 +++ >> 1 file changed, 3 insertions(+) >> >> diff --git a/net/netfilter/nf_tables_api.c >b/net/netfilter/nf_tables_api.c >> index cf767bc58e18..013d28899cab 100644 >> --- a/net/netfilter/nf_tables_api.c >> +++ b/net/netfilter/nf_tables_api.c >> @@ -5140,6 +5140,9 @@ static int nf_tables_updobj(const struct >nft_ctx *ctx, >> struct nft_trans *trans; >> int err; >> >> + if (!obj->ops->update) >> + return -EOPNOTSUPP; >> + >> trans = nft_trans_alloc(ctx, NFT_MSG_NEWOBJ, >> sizeof(struct nft_trans_obj)); >> if (!trans) >> -- >> 2.20.1 > >I think this introduced a regression when adding an object that already >exists: > > # nft add table inet foobar > # nft add counter inet foobar my_counter > # nft add counter inet foobar my_counter > Error: Could not process rule: Operation not supported > add counter inet foobar my_counter > ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ > >It applies to all objects that don't provide an update handler; >counter, >ct helper, ct timeout, ct exception, etc. Hi Eric, It seems that you are right. What would be the behaviour here? Resets the object properties? Thanks Eric! ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH nf-next v2] netfilter: nf_tables: fix possible null-pointer dereference in object update 2019-11-01 15:01 ` Fernando Fernández Mancera @ 2019-11-01 15:11 ` Eric Garver 2019-11-01 15:13 ` Fernando Fernández Mancera 0 siblings, 1 reply; 6+ messages in thread From: Eric Garver @ 2019-11-01 15:11 UTC (permalink / raw) To: Fernando Fernández Mancera; +Cc: netfilter-devel On Fri, Nov 01, 2019 at 04:01:51PM +0100, Fernando Fernández Mancera wrote: > El 1 de noviembre de 2019 15:42:46 CET, Eric Garver <eric@garver.life> escribió: > >Hi Fernando, > > > >On Wed, Sep 04, 2019 at 02:29:07PM +0200, Fernando Fernandez Mancera > >wrote: > >> Not all objects need an update operation. If the object type doesn't > >implement > >> an update operation and the user tries to update it there will be a > >EOPNOTSUPP > >> error instead of a null pointer. > >> > >> Fixes: d62d0ba97b58 ("netfilter: nf_tables: Introduce stateful object > >update operation") > >> Signed-off-by: Fernando Fernandez Mancera <ffmancera@riseup.net> > >> --- > >> net/netfilter/nf_tables_api.c | 3 +++ > >> 1 file changed, 3 insertions(+) > >> > >> diff --git a/net/netfilter/nf_tables_api.c > >b/net/netfilter/nf_tables_api.c > >> index cf767bc58e18..013d28899cab 100644 > >> --- a/net/netfilter/nf_tables_api.c > >> +++ b/net/netfilter/nf_tables_api.c > >> @@ -5140,6 +5140,9 @@ static int nf_tables_updobj(const struct > >nft_ctx *ctx, > >> struct nft_trans *trans; > >> int err; > >> > >> + if (!obj->ops->update) > >> + return -EOPNOTSUPP; > >> + > >> trans = nft_trans_alloc(ctx, NFT_MSG_NEWOBJ, > >> sizeof(struct nft_trans_obj)); > >> if (!trans) > >> -- > >> 2.20.1 > > > >I think this introduced a regression when adding an object that already > >exists: > > > > # nft add table inet foobar > > # nft add counter inet foobar my_counter > > # nft add counter inet foobar my_counter > > Error: Could not process rule: Operation not supported > > add counter inet foobar my_counter > > ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ > > > >It applies to all objects that don't provide an update handler; > >counter, > >ct helper, ct timeout, ct exception, etc. > > Hi Eric, > > It seems that you are right. What would be the behaviour here? Resets the object properties? I don't know what the correct behavior is in the kernel - maybe it silently skips it. i.e. no attempt to update, but returns no error. From a user perspective it should happily accept the re-add. # nft add table inet foobar # nft add counter inet foobar my_counter # nft add counter inet foobar my_counter ** no error ** Unless the "create" verb is used, then we should get an error: # nft create counter inet foobar my_counter Error: Could not process rule: File exists create counter inet foobar my_counter ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH nf-next v2] netfilter: nf_tables: fix possible null-pointer dereference in object update 2019-11-01 15:11 ` Eric Garver @ 2019-11-01 15:13 ` Fernando Fernández Mancera 0 siblings, 0 replies; 6+ messages in thread From: Fernando Fernández Mancera @ 2019-11-01 15:13 UTC (permalink / raw) To: Eric Garver; +Cc: netfilter-devel El 1 de noviembre de 2019 16:11:59 CET, Eric Garver <eric@garver.life> escribió: >On Fri, Nov 01, 2019 at 04:01:51PM +0100, Fernando Fernández Mancera >wrote: >> El 1 de noviembre de 2019 15:42:46 CET, Eric Garver ><eric@garver.life> escribió: >> >Hi Fernando, >> > >> >On Wed, Sep 04, 2019 at 02:29:07PM +0200, Fernando Fernandez Mancera >> >wrote: >> >> Not all objects need an update operation. If the object type >doesn't >> >implement >> >> an update operation and the user tries to update it there will be >a >> >EOPNOTSUPP >> >> error instead of a null pointer. >> >> >> >> Fixes: d62d0ba97b58 ("netfilter: nf_tables: Introduce stateful >object >> >update operation") >> >> Signed-off-by: Fernando Fernandez Mancera <ffmancera@riseup.net> >> >> --- >> >> net/netfilter/nf_tables_api.c | 3 +++ >> >> 1 file changed, 3 insertions(+) >> >> >> >> diff --git a/net/netfilter/nf_tables_api.c >> >b/net/netfilter/nf_tables_api.c >> >> index cf767bc58e18..013d28899cab 100644 >> >> --- a/net/netfilter/nf_tables_api.c >> >> +++ b/net/netfilter/nf_tables_api.c >> >> @@ -5140,6 +5140,9 @@ static int nf_tables_updobj(const struct >> >nft_ctx *ctx, >> >> struct nft_trans *trans; >> >> int err; >> >> >> >> + if (!obj->ops->update) >> >> + return -EOPNOTSUPP; >> >> + >> >> trans = nft_trans_alloc(ctx, NFT_MSG_NEWOBJ, >> >> sizeof(struct nft_trans_obj)); >> >> if (!trans) >> >> -- >> >> 2.20.1 >> > >> >I think this introduced a regression when adding an object that >already >> >exists: >> > >> > # nft add table inet foobar >> > # nft add counter inet foobar my_counter >> > # nft add counter inet foobar my_counter >> > Error: Could not process rule: Operation not supported >> > add counter inet foobar my_counter >> > ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ >> > >> >It applies to all objects that don't provide an update handler; >> >counter, >> >ct helper, ct timeout, ct exception, etc. >> >> Hi Eric, >> >> It seems that you are right. What would be the behaviour here? Resets >the object properties? > >I don't know what the correct behavior is in the kernel - maybe it >silently skips it. i.e. no attempt to update, but returns no error. > >From a user perspective it should happily accept the re-add. > > # nft add table inet foobar > # nft add counter inet foobar my_counter > # nft add counter inet foobar my_counter > ** no error ** > >Unless the "create" verb is used, then we should get an error: > > # nft create counter inet foobar my_counter > Error: Could not process rule: File exists > create counter inet foobar my_counter > ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Sure, I am going to prepare a patch for this. Sorry about the regression. Thanks! ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2019-11-01 15:14 UTC | newest] Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2019-09-04 12:29 [PATCH nf-next v2] netfilter: nf_tables: fix possible null-pointer dereference in object update Fernando Fernandez Mancera 2019-09-05 11:22 ` Pablo Neira Ayuso 2019-11-01 14:42 ` Eric Garver 2019-11-01 15:01 ` Fernando Fernández Mancera 2019-11-01 15:11 ` Eric Garver 2019-11-01 15:13 ` Fernando Fernández Mancera
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).