[PATCH net] ppp: restore uncompressed fallback during CCP negotiation

[PATCH net] ppp: restore uncompressed fallback during CCP negotiation

[Qingfang Deng]

ppp_ccp_peek() enables SC_COMP_RUN when sending a CCP Configure-Ack,
before userspace sets SC_CCP_UP. Data traffic during this window can
reach pad_compress_skb() while CCP is still negotiating. If compression
succeeds, len is positive, but the SC_CCP_UP check fails and the packet
falls through to the error path instead of being sent uncompressed.

Deflate and BSD-Compress have already advanced their compression state
by then. Dropping the packet can desynchronize with the peer's state,
causing decompression errors.

Restore the uncompressed fallback for non-MPPE compressors when len is
positive but CCP is not up. Sending the original packet lets the peer
update its decompression state. Continue to drop packets on negative
compressor returns, and preserve the existing behavior for CI_MPPE so
that encrypted data cannot fall back to plaintext during negotiation.

Fixes: b3f9b92a6ec1 ("[PPP]: add PPP MPPE encryption module")
Reported-by: Sashiko <netdev-bot+sashiko@kernel.org>
Closes: https://lore.kernel.org/netdev/179339596607.3131.10030375792154782283@kernel.org/
Signed-off-by: Qingfang Deng <qingfang.deng@linux.dev>
---
Reproduced on a WSL2 host running 6.18.58 with an async channel and
Deflate configured: the frame sent in the negotiation window was dropped
(tx_errors went from 0 to 1), and with this applied it leaves as an
uncompressed PPP_IP frame and tx_errors stays unchanged. The frame sent
after SC_CCP_UP is set is compressed with sequence number 1, which is
what a peer that fed the uncompressed frame to incomp() expects.

Diagnosis, implementation and local reproduction are assisted by Codex.
---
 drivers/net/ppp/ppp_generic.c | 13 +++++--------
 1 file changed, 5 insertions(+), 8 deletions(-)

diff --git a/drivers/net/ppp/ppp_generic.c b/drivers/net/ppp/ppp_generic.c
index 29847dc4bb45..d3e551a3c80b 100644
--- a/drivers/net/ppp/ppp_generic.c
+++ b/drivers/net/ppp/ppp_generic.c
@@ -1747,18 +1747,15 @@ pad_compress_skb(struct ppp *ppp, struct sk_buff *skb)
 		skb = new_skb;
 		skb_put(skb, len);
 		skb_pull(skb, 2);	/* pull off A/C bytes */
-	} else if (len == 0) {
+	} else if (len == 0 ||
+		   (len > 0 && ppp->xcomp->compress_proto != CI_MPPE)) {
 		/* didn't compress, or CCP not up yet */
 		consume_skb(new_skb);
 		new_skb = skb;
 	} else {
-		/*
-		 * (len < 0)
-		 * MPPE requires that we do not send unencrypted
-		 * frames.  The compressor will return -1 if we
-		 * should drop the frame.  We cannot simply test
-		 * the compress_proto because MPPE and MPPC share
-		 * the same number.
+		/* A negative return means the original packet must not be sent.
+		 * Also prevent MPPE/MPPC from falling back to plaintext while
+		 * CCP is not up.
 		 */
 		if (net_ratelimit())
 			netdev_err(dev, "ppp: compressor dropped pkt\n");
-- 
2.34.1