Re: [PATCH v12 04/11] can: bcm: fix CAN frame rx/tx statistics
| Newsgroups | org.kernel.vger.linux-can |
|---|---|
| Message-ID | <[email protected]> |
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: Pre-existing issues: - [High] Unsynchronized modification of rx_stamp and rx_ifindex allows concurrent frames to corrupt each other's metadata. -- --- Patch [4]: [PATCH v12 04/11] can: bcm: fix CAN frame rx/tx statistics --- commit f0713957841f9a5510dbab52a7bc1b9ca16a367e Author: Oliver Hartkopp <[email protected]> can: bcm: fix CAN frame rx/tx statistics This commit addresses a KCSAN-detected data race in bcm_rx_handler() by converting rx/tx statistics to atomic operations. It also simplifies overflow handling and resolves a race condition where frames_filtered could become larger than frames_abs. > diff --git a/net/can/bcm.c b/net/can/bcm.c > --- a/net/can/bcm.c > +++ b/net/can/bcm.c [ ... ] > @@ -748,24 +767,30 @@ static void bcm_rx_handler(struct sk_buff *skb, void *data) > op->rx_stamp = skb->tstamp; > /* save originator for recvfrom() */ > op->rx_ifindex = skb->dev->ifindex; [Severity: High] This is a pre-existing issue, but does this unsynchronized modification allow concurrent frames to corrupt each other's metadata? When a wildcard (ANYDEV) CAN BCM receive operation is configured, multiple interfaces might receive matching packets simultaneously. Because op->rx_stamp and op->rx_ifindex are updated outside of bcm_rx_update_lock, could different CPUs concurrently overwrite these fields? If so, this might result in userspace receiving packets with mismatched timestamps and source interface indices, potentially causing logic errors in routing or packet processing. > - /* update statistics */ > - op->frames_abs++; > > - /* snapshot the flag under lock: op->flags/op->frames may be updated > - * concurrently by bcm_rx_setup(). > - */ > + /* op->flags/op->frames may be updated concurrently by bcm_rx_setup() */ > spin_lock_bh(&op->bcm_rx_update_lock); [ ... ] -- Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=4