[SRU][J:linux-bluefield][PATCH v1 1/3] UBUNTU: SAUCE: Revert "can: gw: fix RCU/BH usage in cgw_create_job()"
Stav Aviram
saviram at nvidia.com
Thu Jul 17 14:27:02 UTC 2025
BugLink: https://bugs.launchpad.net/bugs/2117163
Revert a commit that was introduced as a part of a series to the stable
release of v5.15.183 and was taken into tag
Ubuntu-bluefield-5.15.0-1071.73.
Remove this series as it added k[v]free_rcu_mightsleep() to BF5.15,
causing HAVE_KFREE_RCU_MIGHTSLEEP to become defined and triggering a
crash due to premature use of the new APIs:
Oops: 0000 [#1] SMP NOPTI
...
Workqueue: events kfree_rcu_work
RIP: 0010:kmem_cache_free_bulk+0x137/0x1d0
...
Call Trace:
kfree_rcu_work+0x1e7/0x250
process_one_work+0x1b0/0x350
worker_thread+0x50/0x3a0
? process_one_work+0x350/0x350
kthread+0x124/0x150
This reverts commit 57818f6fec6c8e702751c49ae7b4b84be84598e8.
Issue: 4530194
Signed-off-by: Stav Aviram <saviram at nvidia.com>
---
net/can/gw.c | 149 ++++++++++++++++++++-------------------------------
1 file changed, 59 insertions(+), 90 deletions(-)
diff --git a/net/can/gw.c b/net/can/gw.c
index c48e8cf5e650..20e74fe7d090 100644
--- a/net/can/gw.c
+++ b/net/can/gw.c
@@ -130,7 +130,7 @@ struct cgw_job {
u32 handled_frames;
u32 dropped_frames;
u32 deleted_frames;
- struct cf_mod __rcu *cf_mod;
+ struct cf_mod mod;
union {
/* CAN frame data source */
struct net_device *dev;
@@ -459,7 +459,6 @@ static void can_can_gw_rcv(struct sk_buff *skb, void *data)
struct cgw_job *gwj = (struct cgw_job *)data;
struct canfd_frame *cf;
struct sk_buff *nskb;
- struct cf_mod *mod;
int modidx = 0;
/* process strictly Classic CAN or CAN FD frames */
@@ -507,8 +506,7 @@ static void can_can_gw_rcv(struct sk_buff *skb, void *data)
* When there is at least one modification function activated,
* we need to copy the skb as we want to modify skb->data.
*/
- mod = rcu_dereference(gwj->cf_mod);
- if (mod->modfunc[0])
+ if (gwj->mod.modfunc[0])
nskb = skb_copy(skb, GFP_ATOMIC);
else
nskb = skb_clone(skb, GFP_ATOMIC);
@@ -531,8 +529,8 @@ static void can_can_gw_rcv(struct sk_buff *skb, void *data)
cf = (struct canfd_frame *)nskb->data;
/* perform preprocessed modification functions if there are any */
- while (modidx < MAX_MODFUNCTIONS && mod->modfunc[modidx])
- (*mod->modfunc[modidx++])(cf, mod);
+ while (modidx < MAX_MODFUNCTIONS && gwj->mod.modfunc[modidx])
+ (*gwj->mod.modfunc[modidx++])(cf, &gwj->mod);
/* Has the CAN frame been modified? */
if (modidx) {
@@ -548,11 +546,11 @@ static void can_can_gw_rcv(struct sk_buff *skb, void *data)
}
/* check for checksum updates */
- if (mod->csumfunc.crc8)
- (*mod->csumfunc.crc8)(cf, &mod->csum.crc8);
+ if (gwj->mod.csumfunc.crc8)
+ (*gwj->mod.csumfunc.crc8)(cf, &gwj->mod.csum.crc8);
- if (mod->csumfunc.xor)
- (*mod->csumfunc.xor)(cf, &mod->csum.xor);
+ if (gwj->mod.csumfunc.xor)
+ (*gwj->mod.csumfunc.xor)(cf, &gwj->mod.csum.xor);
}
/* clear the skb timestamp if not configured the other way */
@@ -583,20 +581,9 @@ static void cgw_job_free_rcu(struct rcu_head *rcu_head)
{
struct cgw_job *gwj = container_of(rcu_head, struct cgw_job, rcu);
- /* cgw_job::cf_mod is always accessed from the same cgw_job object within
- * the same RCU read section. Once cgw_job is scheduled for removal,
- * cf_mod can also be removed without mandating an additional grace period.
- */
- kfree(rcu_access_pointer(gwj->cf_mod));
kmem_cache_free(cgw_cache, gwj);
}
-/* Return cgw_job::cf_mod with RTNL protected section */
-static struct cf_mod *cgw_job_cf_mod(struct cgw_job *gwj)
-{
- return rcu_dereference_protected(gwj->cf_mod, rtnl_is_locked());
-}
-
static int cgw_notifier(struct notifier_block *nb,
unsigned long msg, void *ptr)
{
@@ -629,7 +616,6 @@ static int cgw_put_job(struct sk_buff *skb, struct cgw_job *gwj, int type,
{
struct rtcanmsg *rtcan;
struct nlmsghdr *nlh;
- struct cf_mod *mod;
nlh = nlmsg_put(skb, pid, seq, type, sizeof(*rtcan), flags);
if (!nlh)
@@ -664,83 +650,82 @@ static int cgw_put_job(struct sk_buff *skb, struct cgw_job *gwj, int type,
goto cancel;
}
- mod = cgw_job_cf_mod(gwj);
if (gwj->flags & CGW_FLAGS_CAN_FD) {
struct cgw_fdframe_mod mb;
- if (mod->modtype.and) {
- memcpy(&mb.cf, &mod->modframe.and, sizeof(mb.cf));
- mb.modtype = mod->modtype.and;
+ if (gwj->mod.modtype.and) {
+ memcpy(&mb.cf, &gwj->mod.modframe.and, sizeof(mb.cf));
+ mb.modtype = gwj->mod.modtype.and;
if (nla_put(skb, CGW_FDMOD_AND, sizeof(mb), &mb) < 0)
goto cancel;
}
- if (mod->modtype.or) {
- memcpy(&mb.cf, &mod->modframe.or, sizeof(mb.cf));
- mb.modtype = mod->modtype.or;
+ if (gwj->mod.modtype.or) {
+ memcpy(&mb.cf, &gwj->mod.modframe.or, sizeof(mb.cf));
+ mb.modtype = gwj->mod.modtype.or;
if (nla_put(skb, CGW_FDMOD_OR, sizeof(mb), &mb) < 0)
goto cancel;
}
- if (mod->modtype.xor) {
- memcpy(&mb.cf, &mod->modframe.xor, sizeof(mb.cf));
- mb.modtype = mod->modtype.xor;
+ if (gwj->mod.modtype.xor) {
+ memcpy(&mb.cf, &gwj->mod.modframe.xor, sizeof(mb.cf));
+ mb.modtype = gwj->mod.modtype.xor;
if (nla_put(skb, CGW_FDMOD_XOR, sizeof(mb), &mb) < 0)
goto cancel;
}
- if (mod->modtype.set) {
- memcpy(&mb.cf, &mod->modframe.set, sizeof(mb.cf));
- mb.modtype = mod->modtype.set;
+ if (gwj->mod.modtype.set) {
+ memcpy(&mb.cf, &gwj->mod.modframe.set, sizeof(mb.cf));
+ mb.modtype = gwj->mod.modtype.set;
if (nla_put(skb, CGW_FDMOD_SET, sizeof(mb), &mb) < 0)
goto cancel;
}
} else {
struct cgw_frame_mod mb;
- if (mod->modtype.and) {
- memcpy(&mb.cf, &mod->modframe.and, sizeof(mb.cf));
- mb.modtype = mod->modtype.and;
+ if (gwj->mod.modtype.and) {
+ memcpy(&mb.cf, &gwj->mod.modframe.and, sizeof(mb.cf));
+ mb.modtype = gwj->mod.modtype.and;
if (nla_put(skb, CGW_MOD_AND, sizeof(mb), &mb) < 0)
goto cancel;
}
- if (mod->modtype.or) {
- memcpy(&mb.cf, &mod->modframe.or, sizeof(mb.cf));
- mb.modtype = mod->modtype.or;
+ if (gwj->mod.modtype.or) {
+ memcpy(&mb.cf, &gwj->mod.modframe.or, sizeof(mb.cf));
+ mb.modtype = gwj->mod.modtype.or;
if (nla_put(skb, CGW_MOD_OR, sizeof(mb), &mb) < 0)
goto cancel;
}
- if (mod->modtype.xor) {
- memcpy(&mb.cf, &mod->modframe.xor, sizeof(mb.cf));
- mb.modtype = mod->modtype.xor;
+ if (gwj->mod.modtype.xor) {
+ memcpy(&mb.cf, &gwj->mod.modframe.xor, sizeof(mb.cf));
+ mb.modtype = gwj->mod.modtype.xor;
if (nla_put(skb, CGW_MOD_XOR, sizeof(mb), &mb) < 0)
goto cancel;
}
- if (mod->modtype.set) {
- memcpy(&mb.cf, &mod->modframe.set, sizeof(mb.cf));
- mb.modtype = mod->modtype.set;
+ if (gwj->mod.modtype.set) {
+ memcpy(&mb.cf, &gwj->mod.modframe.set, sizeof(mb.cf));
+ mb.modtype = gwj->mod.modtype.set;
if (nla_put(skb, CGW_MOD_SET, sizeof(mb), &mb) < 0)
goto cancel;
}
}
- if (mod->uid) {
- if (nla_put_u32(skb, CGW_MOD_UID, mod->uid) < 0)
+ if (gwj->mod.uid) {
+ if (nla_put_u32(skb, CGW_MOD_UID, gwj->mod.uid) < 0)
goto cancel;
}
- if (mod->csumfunc.crc8) {
+ if (gwj->mod.csumfunc.crc8) {
if (nla_put(skb, CGW_CS_CRC8, CGW_CS_CRC8_LEN,
- &mod->csum.crc8) < 0)
+ &gwj->mod.csum.crc8) < 0)
goto cancel;
}
- if (mod->csumfunc.xor) {
+ if (gwj->mod.csumfunc.xor) {
if (nla_put(skb, CGW_CS_XOR, CGW_CS_XOR_LEN,
- &mod->csum.xor) < 0)
+ &gwj->mod.csum.xor) < 0)
goto cancel;
}
@@ -1074,7 +1059,7 @@ static int cgw_create_job(struct sk_buff *skb, struct nlmsghdr *nlh,
struct net *net = sock_net(skb->sk);
struct rtcanmsg *r;
struct cgw_job *gwj;
- struct cf_mod *mod;
+ struct cf_mod mod;
struct can_can_gw ccgw;
u8 limhops = 0;
int err = 0;
@@ -1093,48 +1078,37 @@ static int cgw_create_job(struct sk_buff *skb, struct nlmsghdr *nlh,
if (r->gwtype != CGW_TYPE_CAN_CAN)
return -EINVAL;
- mod = kmalloc(sizeof(*mod), GFP_KERNEL);
- if (!mod)
- return -ENOMEM;
-
- err = cgw_parse_attr(nlh, mod, CGW_TYPE_CAN_CAN, &ccgw, &limhops);
+ err = cgw_parse_attr(nlh, &mod, CGW_TYPE_CAN_CAN, &ccgw, &limhops);
if (err < 0)
- goto out_free_cf;
+ return err;
- if (mod->uid) {
+ if (mod.uid) {
ASSERT_RTNL();
/* check for updating an existing job with identical uid */
hlist_for_each_entry(gwj, &net->can.cgw_list, list) {
- struct cf_mod *old_cf;
-
- old_cf = cgw_job_cf_mod(gwj);
- if (old_cf->uid != mod->uid)
+ if (gwj->mod.uid != mod.uid)
continue;
/* interfaces & filters must be identical */
- if (memcmp(&gwj->ccgw, &ccgw, sizeof(ccgw))) {
- err = -EINVAL;
- goto out_free_cf;
- }
+ if (memcmp(&gwj->ccgw, &ccgw, sizeof(ccgw)))
+ return -EINVAL;
- rcu_assign_pointer(gwj->cf_mod, mod);
- kfree_rcu_mightsleep(old_cf);
+ /* update modifications with disabled softirq & quit */
+ local_bh_disable();
+ memcpy(&gwj->mod, &mod, sizeof(mod));
+ local_bh_enable();
return 0;
}
}
/* ifindex == 0 is not allowed for job creation */
- if (!ccgw.src_idx || !ccgw.dst_idx) {
- err = -ENODEV;
- goto out_free_cf;
- }
+ if (!ccgw.src_idx || !ccgw.dst_idx)
+ return -ENODEV;
gwj = kmem_cache_alloc(cgw_cache, GFP_KERNEL);
- if (!gwj) {
- err = -ENOMEM;
- goto out_free_cf;
- }
+ if (!gwj)
+ return -ENOMEM;
gwj->handled_frames = 0;
gwj->dropped_frames = 0;
@@ -1144,7 +1118,7 @@ static int cgw_create_job(struct sk_buff *skb, struct nlmsghdr *nlh,
gwj->limit_hops = limhops;
/* insert already parsed information */
- RCU_INIT_POINTER(gwj->cf_mod, mod);
+ memcpy(&gwj->mod, &mod, sizeof(mod));
memcpy(&gwj->ccgw, &ccgw, sizeof(ccgw));
err = -ENODEV;
@@ -1171,11 +1145,9 @@ static int cgw_create_job(struct sk_buff *skb, struct nlmsghdr *nlh,
if (!err)
hlist_add_head_rcu(&gwj->list, &net->can.cgw_list);
out:
- if (err) {
+ if (err)
kmem_cache_free(cgw_cache, gwj);
-out_free_cf:
- kfree(mod);
- }
+
return err;
}
@@ -1235,22 +1207,19 @@ static int cgw_remove_job(struct sk_buff *skb, struct nlmsghdr *nlh,
/* remove only the first matching entry */
hlist_for_each_entry_safe(gwj, nx, &net->can.cgw_list, list) {
- struct cf_mod *cf_mod;
-
if (gwj->flags != r->flags)
continue;
if (gwj->limit_hops != limhops)
continue;
- cf_mod = cgw_job_cf_mod(gwj);
/* we have a match when uid is enabled and identical */
- if (cf_mod->uid || mod.uid) {
- if (cf_mod->uid != mod.uid)
+ if (gwj->mod.uid || mod.uid) {
+ if (gwj->mod.uid != mod.uid)
continue;
} else {
/* no uid => check for identical modifications */
- if (memcmp(cf_mod, &mod, sizeof(mod)))
+ if (memcmp(&gwj->mod, &mod, sizeof(mod)))
continue;
}
--
2.38.1
More information about the kernel-team
mailing list