[PATCH net] ppp: remove SC_MUST_COMP

[PATCH net] ppp: remove SC_MUST_COMP

[Qingfang Deng]

SC_MUST_COMP accidentally shares the same value with SC_DC_ERROR.
PPPIOCGFLAGS combines configuration flags with transmit and receive
state, so a userspace cannot distinguish them, and a read-modify-write
sequence can promote a decompression error into SC_MUST_COMP.

The flag entered Linux in 2005 with commit b3f9b92a6ec1 ("[PPP]: add PPP
MPPE encryption module"). The original patch submission [1] said pppd
would set it when CCP came up with MPPE enabled. That intended
integration [2] appears never to have landed upstream.

Remove the broken and unused SC_MUST_COMP from the UAPI, SC_MASK and
SC_FLAG_BITS, and remove its transmit and receive drop checks.
PPPIOCSFLAGS will discard this bit while PPPIOCGFLAGS can still report
it as SC_DC_ERROR.

Fixes: b3f9b92a6ec1 ("[PPP]: add PPP MPPE encryption module")
[1] https://lore.kernel.org/netdev/20050814141046.GB20944@lists.us.dell.com/
[2] https://lore.kernel.org/netdev/20050814190529.GA26802@lists.us.dell.com/
Signed-off-by: Qingfang Deng <qingfang.deng@linux.dev>
---
Found and reproduced with LLM. Combined with the broken BSD-Compress on
64-bit, it results in a broken link:

1. First BSD failure: the kernel sets both SC_DC_FERROR and SC_DC_ERROR,
   then passes the failed packet to userspace. pppd closes CCP. Traffic
   falls back to uncompressed, but SC_MUST_COMP becomes accidentally
   latched.

2. Re-enable compression: send SIGUSR2 to pppd, to let BSD compression
   renegotiate. This is a documented pppd operation (man 8 pppd). The
   stale flag survives.

3. Second BSD failure: the kernel sets both error flags again. Now the
   receive check fires:

	if (ppp->flags & SC_MUST_COMP && ppp->rstate & SC_DC_FERROR)
	    goto err;

   The kernel drops the failed packet before delivering it to pppd.
   Consequently, pppd does not perform its normal CCP shutdown and
   cleanup.

4. Subsequent incoming IP and CCP packets hit the same drop check,
   preventing normal recovery.
---
 drivers/net/ppp/ppp_generic.c  | 15 ++-------------
 include/uapi/linux/ppp-ioctl.h |  3 +--
 2 files changed, 3 insertions(+), 15 deletions(-)

diff --git a/drivers/net/ppp/ppp_generic.c b/drivers/net/ppp/ppp_generic.c
index 1a610a18893b..84ca4e93ad9a 100644
--- a/drivers/net/ppp/ppp_generic.c
+++ b/drivers/net/ppp/ppp_generic.c
@@ -157,14 +157,13 @@ struct ppp {
 
 /*
  * Bits in flags: SC_NO_TCP_CCID, SC_CCP_OPEN, SC_CCP_UP, SC_LOOP_TRAFFIC,
- * SC_MULTILINK, SC_MP_SHORTSEQ, SC_MP_XSHORTSEQ, SC_COMP_TCP, SC_REJ_COMP_TCP,
- * SC_MUST_COMP
+ * SC_MULTILINK, SC_MP_SHORTSEQ, SC_MP_XSHORTSEQ, SC_COMP_TCP, SC_REJ_COMP_TCP.
  * Bits in rstate: SC_DECOMP_RUN, SC_DC_ERROR, SC_DC_FERROR.
  * Bits in xstate: SC_COMP_RUN
  */
 #define SC_FLAG_BITS	(SC_NO_TCP_CCID|SC_CCP_OPEN|SC_CCP_UP|SC_LOOP_TRAFFIC \
 			 |SC_MULTILINK|SC_MP_SHORTSEQ|SC_MP_XSHORTSEQ \
-			 |SC_COMP_TCP|SC_REJ_COMP_TCP|SC_MUST_COMP)
+			 |SC_COMP_TCP|SC_REJ_COMP_TCP)
 
 /*
  * Private data structure for each channel.
@@ -1862,13 +1861,6 @@ ppp_prepare_tx_skb(struct ppp *ppp, struct sk_buff **pskb)
 	/* try to do packet compression */
 	if ((ppp->xstate & SC_COMP_RUN) && ppp->xc_state &&
 	    proto != PPP_LCP && proto != PPP_CCP) {
-		if (!(ppp->flags & SC_CCP_UP) && (ppp->flags & SC_MUST_COMP)) {
-			if (net_ratelimit())
-				netdev_err(dev,
-					   "ppp: compression required but "
-					   "down - pkt dropped.\n");
-			goto drop;
-		}
 		new_skb = pad_compress_skb(ppp, skb);
 		if (!new_skb)
 			goto drop;
@@ -2416,9 +2408,6 @@ ppp_receive_nonmp_frame(struct ppp *ppp, struct sk_buff *skb)
 	    (ppp->rstate & (SC_DC_FERROR | SC_DC_ERROR)) == 0)
 		skb = ppp_decompress_frame(ppp, skb);
 
-	if (ppp->flags & SC_MUST_COMP && ppp->rstate & SC_DC_FERROR)
-		goto err;
-
 	/* At this point the "Protocol" field MUST be decompressed, either in
 	 * ppp_input(), ppp_decompress_frame() or in ppp_receive_mp_frame().
 	 */
diff --git a/include/uapi/linux/ppp-ioctl.h b/include/uapi/linux/ppp-ioctl.h
index 1cc5ce0ae062..672f7ac11a0b 100644
--- a/include/uapi/linux/ppp-ioctl.h
+++ b/include/uapi/linux/ppp-ioctl.h
@@ -39,8 +39,7 @@
 #define SC_LOG_RAWIN	0x00080000	/* log all chars received */
 #define SC_LOG_FLUSH	0x00100000	/* log all chars flushed */
 #define	SC_SYNC		0x00200000	/* synchronous serial mode */
-#define	SC_MUST_COMP    0x00400000	/* no uncompressed packets may be sent or received */
-#define	SC_MASK		0x0f600fff	/* bits that user can change */
+#define	SC_MASK		0x0f200fff	/* bits that user can change */
 
 /* state bits */
 #define SC_XMIT_BUSY	0x10000000	/* (used by isdn_ppp?) */
-- 
2.43.0