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

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

[Norbert Szetei]

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);
 		if (copy_to_user(addr, &cstats, sizeof(cstats)))
 			break;
 		err = 0;
@@ -3303,7 +3306,7 @@ find_compressor(int type)
 static void
 ppp_get_stats(struct ppp *ppp, struct ppp_stats *st)
 {
-	struct slcompress *vj = ppp->vj;
+	struct slcompress *vj;
 	int cpu;

 	memset(st, 0, sizeof(*st));
@@ -3323,8 +3326,14 @@ ppp_get_stats(struct ppp *ppp, struct ppp_stats *st)
 	}
 	st->p.ppp_ierrors = ppp->dev->stats.rx_errors;
 	st->p.ppp_oerrors = ppp->dev->stats.tx_errors;
-	if (!vj)
+
+	/* protect against PPPIOCSMAXCID freeing ppp->vj */
+	ppp_recv_lock(ppp);
+	vj = ppp->vj;
+	if (!vj) {
+		ppp_recv_unlock(ppp);
 		return;
+	}
 	st->vj.vjs_packets = vj->sls_o_compressed + vj->sls_o_uncompressed;
 	st->vj.vjs_compressed = vj->sls_o_compressed;
 	st->vj.vjs_searches = vj->sls_o_searches;
@@ -3333,6 +3342,7 @@ ppp_get_stats(struct ppp *ppp, struct ppp_stats *st)
 	st->vj.vjs_tossed = vj->sls_i_tossed;
 	st->vj.vjs_uncompressedin = vj->sls_i_uncompressed;
 	st->vj.vjs_compressedin = vj->sls_i_compressed;
+	ppp_recv_unlock(ppp);
 }

 /*
--
2.54.0

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

[Paolo Abeni]

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