Re: [PATCH] Bluetooth: 6lowpan: Defer peer channel release until RCU cleanup

[c Z]

Hillf Danton <hdanton@xxxxxxxx> 于2026年5月10日周日 09:44写道：
>
> On Sun, 10 May 2026 01:37:45 +0800 Zhang Cen wrote:
> > A 6LoWPAN peer stores the protocol-owned L2CAP channel reference in
> > peer->chan. chan_close_cb() removes the peer from the RCU list, but it
> > also drops that reference immediately. The peer object can still be seen
> > by in-flight RCU readers, and some paths keep using peer->chan after the
> > lookup has finished.
> >
> > That lets transmit and disconnect paths dereference, lock, or send
> > through a channel whose last reference was released by the close path.
> >
> Sounds like the race between close and lookup paths
>
>         chan_close_cb()         setup_header()
>         ---                     ---
>         peer_del()
>           list_del_rcu(&peer->list);
>           kfree_rcu(peer, rcu);
>         l2cap_chan_put(c); // free chan
>
>                                 peer_lookup_dst(dev, &ipv6_daddr, skb);
>                                   rcu_read_lock();
>                                   peer = find peer;
>                                   rcu_read_unlock();
>                                 if (peer)
>                                         addr = peer->chan->dst; //uaf
>
> and on the lookup side, a) peer is unsafe outside rcu lock and
> b) chan is used after put.
>
> To fix the race, in addition to move l2cap_chan_put() to the rcu
> callback as this patch does, refcount should be added to peer to
> make the lookup path safe.
>
>         chan_close_cb()         setup_header()
>         ---                     ---
>         peer_del()
>           list_del_rcu(&peer->list);
>           call_rcu(&peer->rcu, peer_free_rcu);
>             if (peer->refcount-- == 0) {
>                 //alternatively schedule workqueue if task context needed
>                 l2cap_chan_put(c);
>                 kfree(peer);
>                 module_put(THIS_MODULE);
>             }
>                                 peer_lookup_dst(dev, &ipv6_daddr, skb);
>                                   rcu_read_lock();
>                                   peer = find peer;
>                                   if (peer)
>                                         peer->refcount++;
>                                   rcu_read_unlock();
>                                 if (peer) {
>                                         addr = peer->chan->dst;
>                                         if (peer->refcount-- == 0)
>                                                 schedule workqueue to free peer;
>                                 }
>
Hi Hillf,

Thanks for looking at this and for spelling out the lookup-side lifetime
issue.

I agree that v2 should make the peer lifetime explicit. v1 moves the
peer-owned l2cap_chan_put() behind the RCU grace period, and it also
changes the unicast transmit lookup to copy the peer fields / take a
channel reference before dropping RCU. But the broader point still
stands: helpers such as lookup_peer() should not hand a raw lowpan_peer
to users that continue after rcu_read_unlock().

I'll prepare v2 so the peer object has its own refcount. The plan is to:

  * add a refcount/kref to struct lowpan_peer and initialize it with the
    peer-list owner reference;
  * make RCU lookups that return a peer acquire a reference before
    unlocking, using the usual non-zero refcount pattern;
  * make peer_del() only unlink the peer and schedule an RCU callback
    which drops the owner reference after the grace period;
  * release the peer-owned L2CAP channel reference and module reference
    from the final peer put; and
  * audit the remaining lookup_peer()/peer_lookup_dst() users so every
    post-lookup use either holds a peer reference or only uses data copied
    while still under RCU.

Thanks,
Zhang Cen