Re: [PATCH net] ppp: fix use-after-free reads in the stats ioctls.

[Paolo Abeni <pabeni@redhat.com>]

Adding Qingfang.

On 6/28/26 2:44 PM, Norbert Szetei wrote:
> ppp_get_stats() (SIOCGPPPSTATS) and the SIOCGPPPCSTATS handler, both
> reached from ppp_net_siocdevprivate(), dereference state that other
> ioctls free under the ppp lock, without taking it:
> 
>   - ppp_get_stats() reads ppp->vj; PPPIOCSMAXCID frees it with
>     slhc_free() under ppp_lock().
>   - SIOCGPPPCSTATS calls ->comp_stat()/->decomp_stat() on
>     ppp->xc_state / ppp->rc_state; PPPIOCSCOMPRESS and ppp_ccp_closed()
>     free those.
> 
> A concurrent stats ioctl can then read freed memory (slab-use-after-
> free), and the freed contents are copied back to userspace. This is 
> reachable by a local user who has CAP_NET_ADMIN privileges and 
> read/write access to /dev/ppp.
> 
> Take the lock the freeing path holds around each access: the receive
> lock in ppp_get_stats() (PPPIOCSMAXCID frees ppp->vj under ppp_lock(),
> which includes it) and ppp_lock() around the SIOCGPPPCSTATS callbacks.
> 
> Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
> Assisted-by: Claude:claude-opus-4-8
> Signed-off-by: Norbert Szetei <norbert@doyensec.com>
> ---
>  drivers/net/ppp/ppp_generic.c | 14 ++++++++++++--
>  1 file changed, 12 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/net/ppp/ppp_generic.c b/drivers/net/ppp/ppp_generic.c
> index 57c68efa5ff8..847c5e1793c8 100644
> --- a/drivers/net/ppp/ppp_generic.c
> +++ b/drivers/net/ppp/ppp_generic.c
> @@ -1505,10 +1505,13 @@ ppp_net_siocdevprivate(struct net_device *dev, struct ifreq *ifr,
> 
>  	case SIOCGPPPCSTATS:
>  		memset(&cstats, 0, sizeof(cstats));
> +		/* protect against PPPIOCSCOMPRESS/ppp_ccp_closed() freeing the state */
> +		ppp_lock(ppp);
>  		if (ppp->xc_state)
>  			ppp->xcomp->comp_stat(ppp->xc_state, &cstats.c);
>  		if (ppp->rc_state)
>  			ppp->rcomp->decomp_stat(ppp->rc_state, &cstats.d);
> +		ppp_unlock(ppp);

It looks like that this fix addresses the reported races, but I don't
like stats blocking the TX and RX path. Perhaps you should consider
switching to proper RCU for the relevant structs, and likely 2 separate
patches, one for xc_state/rc_state and the other for vj.

/P