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

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

[Fernando Fernandez Mancera]

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, 2 insertions(+), 1 deletion(-)

diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c
index cf767bc58e18..6893de9e1389 100644
--- a/net/netfilter/nf_tables_api.c
+++ b/net/netfilter/nf_tables_api.c
@@ -6477,7 +6477,8 @@ static void nft_obj_commit_update(struct nft_trans *trans)
 	obj = nft_trans_obj(trans);
 	newobj = nft_trans_obj_newobj(trans);
 
-	obj->ops->update(obj, newobj);
+	if (obj->ops->update)
+		obj->ops->update(obj, newobj);
 
 	kfree(newobj);
 }
-- 
2.20.1

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

[Phil Sutter]

Hi Fernando,

On Tue, Sep 03, 2019 at 11:33:13PM +0200, Fernando Fernandez Mancera wrote:
> Fixes: d62d0ba97b58 ("netfilter: nf_tables: Introduce stateful object update operation")
> Signed-off-by: Fernando Fernandez Mancera <ffmancera@riseup.net>

Your patch looks good but please (always) provide a bit of explanation.
In this case typical questions to answer in commit message are:
- Why may obj->ops->update be NULL? For which object type are they not
  defined?
- How could one trigger the issue? In other words, why is
  nft_obj_commit_update() called for the "wrong" object?

Cheers, Phil

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

[Fernando Fernandez Mancera]

Hi Phil,

I am sending a v2 with an explanation. Thanks!

On 9/4/19 8:58 AM, Phil Sutter wrote:
> Hi Fernando,
> 
> On Tue, Sep 03, 2019 at 11:33:13PM +0200, Fernando Fernandez Mancera wrote:
>> Fixes: d62d0ba97b58 ("netfilter: nf_tables: Introduce stateful object update operation")
>> Signed-off-by: Fernando Fernandez Mancera <ffmancera@riseup.net>
> 
> Your patch looks good but please (always) provide a bit of explanation.
> In this case typical questions to answer in commit message are:
> - Why may obj->ops->update be NULL? For which object type are they not
>   defined?
> - How could one trigger the issue? In other words, why is
>   nft_obj_commit_update() called for the "wrong" object?
> 
> Cheers, Phil
> 

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

[Pablo Neira Ayuso]

On Tue, Sep 03, 2019 at 11:33:13PM +0200, Fernando Fernandez Mancera wrote:
> 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, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c
> index cf767bc58e18..6893de9e1389 100644
> --- a/net/netfilter/nf_tables_api.c
> +++ b/net/netfilter/nf_tables_api.c
> @@ -6477,7 +6477,8 @@ static void nft_obj_commit_update(struct nft_trans *trans)
>  	obj = nft_trans_obj(trans);
>  	newobj = nft_trans_obj_newobj(trans);
>  
> -	obj->ops->update(obj, newobj);
> +	if (obj->ops->update)
> +		obj->ops->update(obj, newobj);

Please, check for obj->ops->update() from the preparation phase, ie.
from nf_tables_updobj().

If obj->ops->update is NULL, then return -EOPNOTSUPP.