[PATCH] tty: n_hdlc: fix duplicated prefix on partial writes

[PATCH] tty: n_hdlc: fix duplicated prefix on partial writes

[Cheng Yongkang]

n_hdlc_send_frames() passes the whole queued frame to tty->ops->write().
When the driver accepts only a prefix and returns a positive short count,
the current code puts the whole buffer back at the head of tx_buf_list
without adjusting its length.  The wakeup callback later sends the frame
again from byte 0, so the prefix already committed to the peer's flip
buffer is transmitted twice:

  DUPLICATED PREFIX: magic occurrence 5 at offset 19968, frame index 4
  drained 36864 bytes, target 28672

n_hdlc_tty_write() still returns the full count because the ldisc owns
the queued frame; the short write is an internal transport event.  Drop
the written prefix from the buffer and requeue only the remaining bytes,
so the next wakeup continues where the driver stopped.  Full writes, the
-ERESTARTSYS rollback, other errors, and a zero-byte write keep their
existing behavior.  Clamp a driver report larger than the frame to the
frame length.

Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Cc: stable@vger.kernel.org
Assisted-by: LLM
Signed-off-by: Cheng Yongkang <teel4res@gmail.com>
---
 drivers/tty/n_hdlc.c | 15 +++++++++++++--
 1 file changed, 13 insertions(+), 2 deletions(-)

diff --git a/drivers/tty/n_hdlc.c b/drivers/tty/n_hdlc.c
index 895b850..db2103f 100644
--- a/drivers/tty/n_hdlc.c
+++ b/drivers/tty/n_hdlc.c
@@ -288,6 +288,9 @@ check_again:
 		/* pretending it was accepted by driver */
 		if (actual < 0)
 			actual = tbuf->count;
+		/* a driver cannot have accepted more than the frame */
+		if (actual > tbuf->count)
+			actual = tbuf->count;
 
 		if (actual == tbuf->count) {
 			pr_debug("frame %p completed\n", tbuf);
@@ -304,9 +307,17 @@ check_again:
 			pr_debug("frame %p pending\n", tbuf);
 
 			/*
-			 * the buffer was not accepted by driver,
-			 * return it back into tx queue
+			 * The frame was not accepted in full: return it back
+			 * into tx queue. A positive count means the driver
+			 * took that many bytes of the frame, so drop that
+			 * prefix and retry only the unsent remainder instead
+			 * of transmitting the prefix a second time.
 			 */
+			if (actual > 0) {
+				memmove(tbuf->buf, tbuf->buf + actual,
+					tbuf->count - actual);
+				tbuf->count -= actual;
+			}
 			n_hdlc_buf_return(&n_hdlc->tx_buf_list, tbuf);
 			break;
 		}
-- 
2.34.1

Re: [PATCH] tty: n_hdlc: fix duplicated prefix on partial writes

[sashiko-bot]

> n_hdlc_send_frames() passes the whole queued frame to tty->ops->write().
> When the driver accepts only a prefix and returns a positive short count,
> the current code puts the whole buffer back at the head of tx_buf_list
> without adjusting its length.  The wakeup callback later sends the frame
> again from byte 0, so the prefix already committed to the peer's flip
> buffer is transmitted twice:
> 
>   DUPLICATED PREFIX: magic occurrence 5 at offset 19968, frame index 4
>   drained 36864 bytes, target 28672
> 
> n_hdlc_tty_write() still returns the full count because the ldisc owns
> the queued frame; the short write is an internal transport event.  Drop
> the written prefix from the buffer and requeue only the remaining bytes,
> so the next wakeup continues where the driver stopped.  Full writes, the
> -ERESTARTSYS rollback, other errors, and a zero-byte write keep their
> existing behavior.  Clamp a driver report larger than the frame to the
> frame length.
> 
> Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
> Cc: stable@vger.kernel.org
> Assisted-by: LLM
> Signed-off-by: Cheng Yongkang <teel4res@gmail.com>

Sashiko has reviewed this patch and found no issues.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/179328314077.648427.13745367584859897488@gmail.com?part=1