lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <Z685fovQy0yL6stZ@mev-dev.igk.intel.com>
Date: Fri, 14 Feb 2025 13:43:03 +0100
From: Michal Swiatkowski <michal.swiatkowski@...ux.intel.com>
To: Pierre Riteau <pierre@...ckhpc.com>
Cc: netdev@...r.kernel.org
Subject: Re: [PATCH net] net/sched: cls_api: fix error handling causing NULL
 dereference

On Thu, Feb 13, 2025 at 11:36:10PM +0100, Pierre Riteau wrote:
> tcf_exts_miss_cookie_base_alloc() calls xa_alloc_cyclic() which can
> return 1 if the allocation succeeded after wrapping. This was treated as
> an error, with value 1 returned to caller tcf_exts_init_ex() which sets
> exts->actions to NULL and returns 1 to caller fl_change().
> 
> fl_change() treats err == 1 as success, calling tcf_exts_validate_ex()
> which calls tcf_action_init() with exts->actions as argument, where it
> is dereferenced.
> 
> Example trace:
> 
> BUG: kernel NULL pointer dereference, address: 0000000000000000
> CPU: 114 PID: 16151 Comm: handler114 Kdump: loaded Not tainted 5.14.0-503.16.1.el9_5.x86_64 #1
> RIP: 0010:tcf_action_init+0x1f8/0x2c0
> Call Trace:
>  tcf_action_init+0x1f8/0x2c0
>  tcf_exts_validate_ex+0x175/0x190
>  fl_change+0x537/0x1120 [cls_flower]
> 
> Fixes: 80cd22c35c90 ("net/sched: cls_api: Support hardware miss to tc action")
> Signed-off-by: Pierre Riteau <pierre@...ckhpc.com>
> ---
>  net/sched/cls_api.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/net/sched/cls_api.c b/net/sched/cls_api.c
> index 8e47e5355be6..4f648af8cfaa 100644
> --- a/net/sched/cls_api.c
> +++ b/net/sched/cls_api.c
> @@ -97,7 +97,7 @@ tcf_exts_miss_cookie_base_alloc(struct tcf_exts *exts, struct tcf_proto *tp,
>  
>  	err = xa_alloc_cyclic(&tcf_exts_miss_cookies_xa, &n->miss_cookie_base,
>  			      n, xa_limit_32b, &next, GFP_KERNEL);
> -	if (err)
> +	if (err < 0)
>  		goto err_xa_alloc;
>  
>  	exts->miss_cookie_node = n;

Thanks for fixing.
Reviewed-by: Michal Swiatkowski <michal.swiatkowski@...ux.intel.com>

The same thing is done in devlink_rel_alloc() (net/devlink/core.c). I am
not sure if it can lead to NULL pointer dereference as here.

Thanks,
Michal

> -- 
> 2.43.5

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ