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

[Sebastian Andrzej Siewior <bigeasy@linutronix.de>]

On 2026-07-14 10:47:58 [-0700], Paul E. McKenney wrote:
> 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.

Regarding srcu, module unload should have cleanup_srcu_struct() which
would flush pending item, no?

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

Right. What people often did wrong with timers is that they enqueued a
timer then forgot about it and released the memory with the timer
active. Then someone who got all the timer bugs assigned came up with
debugobjects.

This checks only that the memory is not released while the object is
active. It does not verify the callback.

I think it depends on what we want to achieve. We could iterate over all
RCU callbacks checking if it belongs to the memory that we intend to
free. The same could be done for timers, too. They should be caught by
debugbjects unless they leak the memory…

The rcu_barrier() before module_exit() would ensure the callbacks are
done before a possible kmem_cache_free() where the cache is removed in
the exit function. Now I see that for !SLUB_TINY there is a barrier if
the cache has sheaves (kmem_cache_destroy() ->
kvfree_rcu_barrier_on_cache()). Buh…

> 							Thanx, Paul

Sebastian