Re: [PATCH nf-next v2] netfilter: nf_tables: fix possible null-pointer dereference in object update

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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@xxxxxxxxxxx> 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@xxxxxxxxxx>
> >> ---
> >>  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
    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^




[Index of Archives]     [Netfitler Users]     [Berkeley Packet Filter]     [LARTC]     [Bugtraq]     [Yosemite Forum]

  Powered by Linux