* [PATCH] net/sched: Fix UAF when resolving a clash
@ 2024-07-04 9:34 Chengen Du
2024-07-04 10:57 ` Michal Kubiak
0 siblings, 1 reply; 4+ messages in thread
From: Chengen Du @ 2024-07-04 9:34 UTC (permalink / raw)
To: jhs
Cc: xiyou.wangcong, jiri, davem, edumazet, kuba, pabeni, ozsh, paulb,
marcelo.leitner, netdev, linux-kernel, Chengen Du, Gerald Yang
KASAN reports the following UAF:
BUG: KASAN: slab-use-after-free in tcf_ct_flow_table_process_conn+0x12b/0x380 [act_ct]
Read of size 1 at addr ffff888c07603600 by task handler130/6469
Call Trace:
<IRQ>
dump_stack_lvl+0x48/0x70
print_address_description.constprop.0+0x33/0x3d0
print_report+0xc0/0x2b0
kasan_report+0xd0/0x120
__asan_load1+0x6c/0x80
tcf_ct_flow_table_process_conn+0x12b/0x380 [act_ct]
tcf_ct_act+0x886/0x1350 [act_ct]
tcf_action_exec+0xf8/0x1f0
fl_classify+0x355/0x360 [cls_flower]
__tcf_classify+0x1fd/0x330
tcf_classify+0x21c/0x3c0
sch_handle_ingress.constprop.0+0x2c5/0x500
__netif_receive_skb_core.constprop.0+0xb25/0x1510
__netif_receive_skb_list_core+0x220/0x4c0
netif_receive_skb_list_internal+0x446/0x620
napi_complete_done+0x157/0x3d0
gro_cell_poll+0xcf/0x100
__napi_poll+0x65/0x310
net_rx_action+0x30c/0x5c0
__do_softirq+0x14f/0x491
__irq_exit_rcu+0x82/0xc0
irq_exit_rcu+0xe/0x20
common_interrupt+0xa1/0xb0
</IRQ>
<TASK>
asm_common_interrupt+0x27/0x40
Allocated by task 6469:
kasan_save_stack+0x38/0x70
kasan_set_track+0x25/0x40
kasan_save_alloc_info+0x1e/0x40
__kasan_krealloc+0x133/0x190
krealloc+0xaa/0x130
nf_ct_ext_add+0xed/0x230 [nf_conntrack]
tcf_ct_act+0x1095/0x1350 [act_ct]
tcf_action_exec+0xf8/0x1f0
fl_classify+0x355/0x360 [cls_flower]
__tcf_classify+0x1fd/0x330
tcf_classify+0x21c/0x3c0
sch_handle_ingress.constprop.0+0x2c5/0x500
__netif_receive_skb_core.constprop.0+0xb25/0x1510
__netif_receive_skb_list_core+0x220/0x4c0
netif_receive_skb_list_internal+0x446/0x620
napi_complete_done+0x157/0x3d0
gro_cell_poll+0xcf/0x100
__napi_poll+0x65/0x310
net_rx_action+0x30c/0x5c0
__do_softirq+0x14f/0x491
Freed by task 6469:
kasan_save_stack+0x38/0x70
kasan_set_track+0x25/0x40
kasan_save_free_info+0x2b/0x60
____kasan_slab_free+0x180/0x1f0
__kasan_slab_free+0x12/0x30
slab_free_freelist_hook+0xd2/0x1a0
__kmem_cache_free+0x1a2/0x2f0
kfree+0x78/0x120
nf_conntrack_free+0x74/0x130 [nf_conntrack]
nf_ct_destroy+0xb2/0x140 [nf_conntrack]
__nf_ct_resolve_clash+0x529/0x5d0 [nf_conntrack]
nf_ct_resolve_clash+0xf6/0x490 [nf_conntrack]
__nf_conntrack_confirm+0x2c6/0x770 [nf_conntrack]
tcf_ct_act+0x12ad/0x1350 [act_ct]
tcf_action_exec+0xf8/0x1f0
fl_classify+0x355/0x360 [cls_flower]
__tcf_classify+0x1fd/0x330
tcf_classify+0x21c/0x3c0
sch_handle_ingress.constprop.0+0x2c5/0x500
__netif_receive_skb_core.constprop.0+0xb25/0x1510
__netif_receive_skb_list_core+0x220/0x4c0
netif_receive_skb_list_internal+0x446/0x620
napi_complete_done+0x157/0x3d0
gro_cell_poll+0xcf/0x100
__napi_poll+0x65/0x310
net_rx_action+0x30c/0x5c0
__do_softirq+0x14f/0x491
The ct may be dropped if a clash has been resolved but is still passed to
the tcf_ct_flow_table_process_conn function for further usage. This issue
can be fixed by retrieving ct from skb again after confirming conntrack.
Fixes: 0cc254e5aa37 ("net/sched: act_ct: Offload connections with commit action")
Co-developed-by: Gerald Yang <gerald.yang@canonical.com>
Signed-off-by: Gerald Yang <gerald.yang@canonical.com>
Signed-off-by: Chengen Du <chengen.du@canonical.com>
---
net/sched/act_ct.c | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/net/sched/act_ct.c b/net/sched/act_ct.c
index 2a96d9c1db65..079562f6ca71 100644
--- a/net/sched/act_ct.c
+++ b/net/sched/act_ct.c
@@ -1077,6 +1077,13 @@ TC_INDIRECT_SCOPE int tcf_ct_act(struct sk_buff *skb, const struct tc_action *a,
*/
if (nf_conntrack_confirm(skb) != NF_ACCEPT)
goto drop;
+ /* The ct may be dropped if a clash has been resolved,
+ * so it's necessary to retrieve it from skb again to
+ * prevent UAF.
+ */
+ ct = nf_ct_get(skb, &ctinfo);
+ if (!ct)
+ goto drop;
}
if (!skip_add)
--
2.43.0
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] net/sched: Fix UAF when resolving a clash
2024-07-04 9:34 [PATCH] net/sched: Fix UAF when resolving a clash Chengen Du
@ 2024-07-04 10:57 ` Michal Kubiak
2024-07-04 14:30 ` Jakub Kicinski
0 siblings, 1 reply; 4+ messages in thread
From: Michal Kubiak @ 2024-07-04 10:57 UTC (permalink / raw)
To: Chengen Du
Cc: jhs, xiyou.wangcong, jiri, davem, edumazet, kuba, pabeni, ozsh,
paulb, marcelo.leitner, netdev, linux-kernel, Gerald Yang
On Thu, Jul 04, 2024 at 05:34:58PM +0800, Chengen Du wrote:
> KASAN reports the following UAF:
>
> BUG: KASAN: slab-use-after-free in tcf_ct_flow_table_process_conn+0x12b/0x380 [act_ct]
> Read of size 1 at addr ffff888c07603600 by task handler130/6469
>
> Call Trace:
> <IRQ>
> dump_stack_lvl+0x48/0x70
> print_address_description.constprop.0+0x33/0x3d0
> print_report+0xc0/0x2b0
> kasan_report+0xd0/0x120
> __asan_load1+0x6c/0x80
> tcf_ct_flow_table_process_conn+0x12b/0x380 [act_ct]
> tcf_ct_act+0x886/0x1350 [act_ct]
> tcf_action_exec+0xf8/0x1f0
> fl_classify+0x355/0x360 [cls_flower]
> __tcf_classify+0x1fd/0x330
> tcf_classify+0x21c/0x3c0
> sch_handle_ingress.constprop.0+0x2c5/0x500
> __netif_receive_skb_core.constprop.0+0xb25/0x1510
> __netif_receive_skb_list_core+0x220/0x4c0
> netif_receive_skb_list_internal+0x446/0x620
> napi_complete_done+0x157/0x3d0
> gro_cell_poll+0xcf/0x100
> __napi_poll+0x65/0x310
> net_rx_action+0x30c/0x5c0
> __do_softirq+0x14f/0x491
> __irq_exit_rcu+0x82/0xc0
> irq_exit_rcu+0xe/0x20
> common_interrupt+0xa1/0xb0
> </IRQ>
> <TASK>
> asm_common_interrupt+0x27/0x40
>
> Allocated by task 6469:
> kasan_save_stack+0x38/0x70
> kasan_set_track+0x25/0x40
> kasan_save_alloc_info+0x1e/0x40
> __kasan_krealloc+0x133/0x190
> krealloc+0xaa/0x130
> nf_ct_ext_add+0xed/0x230 [nf_conntrack]
> tcf_ct_act+0x1095/0x1350 [act_ct]
> tcf_action_exec+0xf8/0x1f0
> fl_classify+0x355/0x360 [cls_flower]
> __tcf_classify+0x1fd/0x330
> tcf_classify+0x21c/0x3c0
> sch_handle_ingress.constprop.0+0x2c5/0x500
> __netif_receive_skb_core.constprop.0+0xb25/0x1510
> __netif_receive_skb_list_core+0x220/0x4c0
> netif_receive_skb_list_internal+0x446/0x620
> napi_complete_done+0x157/0x3d0
> gro_cell_poll+0xcf/0x100
> __napi_poll+0x65/0x310
> net_rx_action+0x30c/0x5c0
> __do_softirq+0x14f/0x491
>
> Freed by task 6469:
> kasan_save_stack+0x38/0x70
> kasan_set_track+0x25/0x40
> kasan_save_free_info+0x2b/0x60
> ____kasan_slab_free+0x180/0x1f0
> __kasan_slab_free+0x12/0x30
> slab_free_freelist_hook+0xd2/0x1a0
> __kmem_cache_free+0x1a2/0x2f0
> kfree+0x78/0x120
> nf_conntrack_free+0x74/0x130 [nf_conntrack]
> nf_ct_destroy+0xb2/0x140 [nf_conntrack]
> __nf_ct_resolve_clash+0x529/0x5d0 [nf_conntrack]
> nf_ct_resolve_clash+0xf6/0x490 [nf_conntrack]
> __nf_conntrack_confirm+0x2c6/0x770 [nf_conntrack]
> tcf_ct_act+0x12ad/0x1350 [act_ct]
> tcf_action_exec+0xf8/0x1f0
> fl_classify+0x355/0x360 [cls_flower]
> __tcf_classify+0x1fd/0x330
> tcf_classify+0x21c/0x3c0
> sch_handle_ingress.constprop.0+0x2c5/0x500
> __netif_receive_skb_core.constprop.0+0xb25/0x1510
> __netif_receive_skb_list_core+0x220/0x4c0
> netif_receive_skb_list_internal+0x446/0x620
> napi_complete_done+0x157/0x3d0
> gro_cell_poll+0xcf/0x100
> __napi_poll+0x65/0x310
> net_rx_action+0x30c/0x5c0
> __do_softirq+0x14f/0x491
>
> The ct may be dropped if a clash has been resolved but is still passed to
> the tcf_ct_flow_table_process_conn function for further usage. This issue
> can be fixed by retrieving ct from skb again after confirming conntrack.
>
> Fixes: 0cc254e5aa37 ("net/sched: act_ct: Offload connections with commit action")
> Co-developed-by: Gerald Yang <gerald.yang@canonical.com>
> Signed-off-by: Gerald Yang <gerald.yang@canonical.com>
> Signed-off-by: Chengen Du <chengen.du@canonical.com>
> ---
> net/sched/act_ct.c | 7 +++++++
> 1 file changed, 7 insertions(+)
>
> diff --git a/net/sched/act_ct.c b/net/sched/act_ct.c
> index 2a96d9c1db65..079562f6ca71 100644
> --- a/net/sched/act_ct.c
> +++ b/net/sched/act_ct.c
> @@ -1077,6 +1077,13 @@ TC_INDIRECT_SCOPE int tcf_ct_act(struct sk_buff *skb, const struct tc_action *a,
> */
> if (nf_conntrack_confirm(skb) != NF_ACCEPT)
> goto drop;
Nitpick: I would add a newline character before the comment, as that seems
to be the convention in this file for other comments.
> + /* The ct may be dropped if a clash has been resolved,
> + * so it's necessary to retrieve it from skb again to
> + * prevent UAF.
> + */
> + ct = nf_ct_get(skb, &ctinfo);
> + if (!ct)
> + goto drop;
> }
>
> if (!skip_add)
> --
> 2.43.0
>
>
The fix itself looks correct to me.
However, there is no explicit tag where the patch is addressed. It
should be "net" tree as this is a fix. It should look like:
[PATCH net]
Please check the patchwork warning for details.
Thanks,
Michal
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] net/sched: Fix UAF when resolving a clash
2024-07-04 10:57 ` Michal Kubiak
@ 2024-07-04 14:30 ` Jakub Kicinski
2024-07-04 14:46 ` Michal Kubiak
0 siblings, 1 reply; 4+ messages in thread
From: Jakub Kicinski @ 2024-07-04 14:30 UTC (permalink / raw)
To: Michal Kubiak
Cc: Chengen Du, jhs, xiyou.wangcong, jiri, davem, edumazet, pabeni,
ozsh, paulb, marcelo.leitner, netdev, linux-kernel, Gerald Yang
On Thu, 4 Jul 2024 12:57:35 +0200 Michal Kubiak wrote:
> Please check the patchwork warning for details.
Please don't direct people to patchwork checks, it's not a public CI.
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] net/sched: Fix UAF when resolving a clash
2024-07-04 14:30 ` Jakub Kicinski
@ 2024-07-04 14:46 ` Michal Kubiak
0 siblings, 0 replies; 4+ messages in thread
From: Michal Kubiak @ 2024-07-04 14:46 UTC (permalink / raw)
To: Jakub Kicinski
Cc: Chengen Du, jhs, xiyou.wangcong, jiri, davem, edumazet, pabeni,
ozsh, paulb, marcelo.leitner, netdev, linux-kernel, Gerald Yang
On Thu, Jul 04, 2024 at 07:30:59AM -0700, Jakub Kicinski wrote:
> On Thu, 4 Jul 2024 12:57:35 +0200 Michal Kubiak wrote:
> > Please check the patchwork warning for details.
>
> Please don't direct people to patchwork checks, it's not a public CI.
My apologies! I wasn't aware of this rule.
Michal
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2024-07-04 14:46 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-07-04 9:34 [PATCH] net/sched: Fix UAF when resolving a clash Chengen Du
2024-07-04 10:57 ` Michal Kubiak
2024-07-04 14:30 ` Jakub Kicinski
2024-07-04 14:46 ` Michal Kubiak
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).