netdev - Re: [PATCH net-next] net/sched: clear actions pointer in miss cookie init fail

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: <54d9bd03-66b7-649f-ce72-84d08b833d50@mojatatu.com>
Date:   Fri, 14 Apr 2023 18:44:58 -0300
From:   Pedro Tammela <pctammela@...atatu.com>
To:     netdev@...r.kernel.org
Cc:     jhs@...atatu.com, xiyou.wangcong@...il.com, jiri@...nulli.us,
        davem@...emloft.net, edumazet@...gle.com, kuba@...nel.org,
        pabeni@...hat.com, marcelo.leitner@...il.com, paulb@...dia.com,
        simon.horman@...igine.com, Palash Oswal <oswalpalash@...il.com>
Subject: Re: [PATCH net-next] net/sched: clear actions pointer in miss cookie
 init fail

On 14/04/2023 18:43, Pedro Tammela wrote:
> Palash reports a UAF when using a modified version of syzkaller[1].
> 
> When 'tcf_exts_miss_cookie_base_alloc()' fails in 'tcf_exts_init_ex()'
> a call to 'tcf_exts_destroy()' is made to free up the tcf_exts
> resources.
> In flower, a call to '__fl_put()' when 'tcf_exts_init_ex()' fails is made;
> Then calling 'tcf_exts_destroy()', which triggers an UAF since the
> already freed tcf_exts action pointer is lingering in the struct.
> 
> Before the offending patch, this was not an issue since there was no
> case where the tcf_exts action pointer could linger. Therefore, restore
> the old semantic by clearing the action pointer in case of a failure to
> initialize the miss_cookie.
> 
> [1] https://github.com/cmu-pasta/linux-kernel-enriched-corpus
> 
> Cc: paulb@...dia.com
> Fixes: 80cd22c35c90 ("net/sched: cls_api: Support hardware miss to tc action")
> Reported-by: Palash Oswal <oswalpalash@...il.com>
> Acked-by: Jamal Hadi Salim <jhs@...atatu.com>
> Signed-off-by: Pedro Tammela <pctammela@...atatu.com>
> ---
>   net/sched/cls_api.c | 1 +
>   1 file changed, 1 insertion(+)
> 
> diff --git a/net/sched/cls_api.c b/net/sched/cls_api.c
> index 2a6b6be0811b..84bad268e328 100644
> --- a/net/sched/cls_api.c
> +++ b/net/sched/cls_api.c
> @@ -3235,6 +3235,7 @@ int tcf_exts_init_ex(struct tcf_exts *exts, struct net *net, int action,
>   
>   err_miss_alloc:
>   	tcf_exts_destroy(exts);
> +	exts->actions = NULL;
>   	return err;
>   }
>   EXPORT_SYMBOL(tcf_exts_init_ex);


My bad, this should target 'net' instead of 'net-next'.