mirror of
git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
synced 2026-03-21 23:16:50 +08:00
clsact: Fix use-after-free in init/destroy rollback asymmetry
Fix a use-after-free in the clsact qdisc upon init/destroy rollback asymmetry. The latter is achieved by first fully initializing a clsact instance, and then in a second step having a replacement failure for the new clsact qdisc instance. clsact_init() initializes ingress first and then takes care of the egress part. This can fail midway, for example, via tcf_block_get_ext(). Upon failure, the kernel will trigger the clsact_destroy() callback. Commit1cb6f0bae5("bpf: Fix too early release of tcx_entry") details the way how the transition is happening. If tcf_block_get_ext on the q->ingress_block ends up failing, we took the tcx_miniq_inc reference count on the ingress side, but not yet on the egress side. clsact_destroy() tests whether the {ingress,egress}_entry was non-NULL. However, even in midway failure on the replacement, both are in fact non-NULL with a valid egress_entry from the previous clsact instance. What we really need to test for is whether the qdisc instance-specific ingress or egress side previously got initialized. This adds a small helper for checking the miniq initialization called mini_qdisc_pair_inited, and utilizes that upon clsact_destroy() in order to fix the use-after-free scenario. Convert the ingress_destroy() side as well so both are consistent to each other. Fixes:1cb6f0bae5("bpf: Fix too early release of tcx_entry") Reported-by: Keenan Dong <keenanat2000@gmail.com> Signed-off-by: Daniel Borkmann <daniel@iogearbox.net> Cc: Martin KaFai Lau <martin.lau@kernel.org> Acked-by: Martin KaFai Lau <martin.lau@kernel.org> Link: https://patch.msgid.link/20260313065531.98639-1-daniel@iogearbox.net Signed-off-by: Paolo Abeni <pabeni@redhat.com>
This commit is contained in:
committed by
Paolo Abeni
parent
e4c00ba727
commit
a0671125d4
@@ -1457,6 +1457,11 @@ void mini_qdisc_pair_init(struct mini_Qdisc_pair *miniqp, struct Qdisc *qdisc,
|
||||
void mini_qdisc_pair_block_init(struct mini_Qdisc_pair *miniqp,
|
||||
struct tcf_block *block);
|
||||
|
||||
static inline bool mini_qdisc_pair_inited(struct mini_Qdisc_pair *miniqp)
|
||||
{
|
||||
return !!miniqp->p_miniq;
|
||||
}
|
||||
|
||||
void mq_change_real_num_tx(struct Qdisc *sch, unsigned int new_real_tx);
|
||||
|
||||
int sch_frag_xmit_hook(struct sk_buff *skb, int (*xmit)(struct sk_buff *skb));
|
||||
|
||||
@@ -113,14 +113,15 @@ static void ingress_destroy(struct Qdisc *sch)
|
||||
{
|
||||
struct ingress_sched_data *q = qdisc_priv(sch);
|
||||
struct net_device *dev = qdisc_dev(sch);
|
||||
struct bpf_mprog_entry *entry = rtnl_dereference(dev->tcx_ingress);
|
||||
struct bpf_mprog_entry *entry;
|
||||
|
||||
if (sch->parent != TC_H_INGRESS)
|
||||
return;
|
||||
|
||||
tcf_block_put_ext(q->block, sch, &q->block_info);
|
||||
|
||||
if (entry) {
|
||||
if (mini_qdisc_pair_inited(&q->miniqp)) {
|
||||
entry = rtnl_dereference(dev->tcx_ingress);
|
||||
tcx_miniq_dec(entry);
|
||||
if (!tcx_entry_is_active(entry)) {
|
||||
tcx_entry_update(dev, NULL, true);
|
||||
@@ -290,10 +291,9 @@ static int clsact_init(struct Qdisc *sch, struct nlattr *opt,
|
||||
|
||||
static void clsact_destroy(struct Qdisc *sch)
|
||||
{
|
||||
struct bpf_mprog_entry *ingress_entry, *egress_entry;
|
||||
struct clsact_sched_data *q = qdisc_priv(sch);
|
||||
struct net_device *dev = qdisc_dev(sch);
|
||||
struct bpf_mprog_entry *ingress_entry = rtnl_dereference(dev->tcx_ingress);
|
||||
struct bpf_mprog_entry *egress_entry = rtnl_dereference(dev->tcx_egress);
|
||||
|
||||
if (sch->parent != TC_H_CLSACT)
|
||||
return;
|
||||
@@ -301,7 +301,8 @@ static void clsact_destroy(struct Qdisc *sch)
|
||||
tcf_block_put_ext(q->ingress_block, sch, &q->ingress_block_info);
|
||||
tcf_block_put_ext(q->egress_block, sch, &q->egress_block_info);
|
||||
|
||||
if (ingress_entry) {
|
||||
if (mini_qdisc_pair_inited(&q->miniqp_ingress)) {
|
||||
ingress_entry = rtnl_dereference(dev->tcx_ingress);
|
||||
tcx_miniq_dec(ingress_entry);
|
||||
if (!tcx_entry_is_active(ingress_entry)) {
|
||||
tcx_entry_update(dev, NULL, true);
|
||||
@@ -309,7 +310,8 @@ static void clsact_destroy(struct Qdisc *sch)
|
||||
}
|
||||
}
|
||||
|
||||
if (egress_entry) {
|
||||
if (mini_qdisc_pair_inited(&q->miniqp_egress)) {
|
||||
egress_entry = rtnl_dereference(dev->tcx_egress);
|
||||
tcx_miniq_dec(egress_entry);
|
||||
if (!tcx_entry_is_active(egress_entry)) {
|
||||
tcx_entry_update(dev, NULL, false);
|
||||
|
||||
Reference in New Issue
Block a user