Re: [PATCH net v2] ppp: defer channel free to an RCU grace period to fix pppol2tp RX UAF

On Tue, Jul 14, 2026 at 06:01:04PM +0200, Sebastian Andrzej Siewior wrote:
> On 2026-07-08 12:22:58 [-0700], Paul E. McKenney wrote:
> > The RCU-callback ordering guarantees are quite weak:
> …
> So this is not working.
> 
> > What you maybe *could* do is to have the two RCU callbacks communicate,
> > so that the last one to be invoked did the work of both of them.  For
> > example, use a shared variable initialized to 2, then have each callback
> > do atomic_dec_and_test(), with the "winner" doing the work.
> > 
> > Would that do the trick?
> 
> This would work but we would have to fix each one or make it slower for
> everyone in the common case. So I think adding a rcu_barrier() to module
> unload wouldn't be that bad and Petr did not say "get out" so ;)

I am not opposed to rcu_barrier() in module unload.

But just to fill out the other similar situations...

In addition to call_rcu(), there is call_srcu(), call_rcu_tasks(),
and call_rcu_tasks_rude().  I think that we can rule out call_srcu()
as ridiculous because there could have been an arbitrarily large number
of srcu_struct structures passed to call_srcu() along with callback
functions defined within this module.  For their parts, call_rcu_tasks()
and call_rcu_tasks_rude() are special-purpose functions that are rarely
used and could therefore be ignored.

But what about things like timers and hrtimers?  Or are people already
doing the right thing for such things?

> I've been looking a bit more:
> - fs/netfs/objects.c
>   call_rcu(&rreq->rcu, netfs_free_request_rcu). No rcu_barrier or
>   flush_worker. 
> 
> - net/sched/sch_taprio.c
>   call_rcu(&oper->rcu, taprio_free_sched_cb) from with the destroy
>   callback. There is a rcu_barrier() but for classification not qdisc.
> 
> - drivers/net/tun.c.
>   tun_free_netdev() from the net_device::priv_destructor.
> 
> The net things _might_ be okay since there is netdev_run_todo() with a
> rcu_barrier() which is run at rtnl_unlock(). So it might work.
> 
> - drivers/vfio/pci/vfio_pci_core.c
>   call_rcu() but I don't see anything rcu_barrier() like.
> 
> - security/keys/trusted-keys/trusted_core.c 
>   security/keys/encrypted-keys/encrypted.c
>   call_rcu() from an update callback. No rcu_barrier in security/.
>   This looks "easy".
> 
> - drivers/gpu/drm/lima/lima_sched.c
>   lima_fence_release() does call_rcu() from an release callback. Don't
>   see anything that would enforce a rcu barrier here. The remove path
>   has also a lima_sched_slab_fini() which removes the slab cache used in
>   the rcu callback.
> 
> And we had the infinibad thing which got fixed in the meantime.
> So I *think* this qualifies for an rcu_barrier() in the module unload
> path. No objections I guess?

No objection, just calling attention to the possible follow-on to this
particular camel's nose.

							Thanx, Paul