[PATCH 1/5] net: cs8900: simplify buffer read loop

[PATCH 1/5] net: cs8900: simplify buffer read loop

[Sascha Hauer]

Instead of doing an extra read after the buffer read loop when the
packet length is odd, add another iteration to the loop if necessary.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 drivers/net/cs8900.c | 7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/drivers/net/cs8900.c b/drivers/net/cs8900.c
index 0dfd3f1303..afb0f3e26e 100644
--- a/drivers/net/cs8900.c
+++ b/drivers/net/cs8900.c
@@ -295,12 +295,9 @@ static int cs8900_recv(struct eth_device *dev)
 	status = readw(priv->regs + CS8900_RTDATA0);
 	len = readw(priv->regs + CS8900_RTDATA0);
 
-	for (addr = (u16 *)priv->rx_buf, i = len >> 1; i > 0; i--) {
+	for (addr = (u16 *)priv->rx_buf, i = (len + 1) >> 1; i > 0; i--)
 		*addr++ = readw(priv->regs + CS8900_RTDATA0);
-	}
-	if (len & 1) {
-		*addr++ = readw(priv->regs + CS8900_RTDATA0);
-	}
+
 	net_receive(dev, priv->rx_buf, len);
 
 	return len;
-- 
2.39.2

[PATCH 2/5] net: cs8900: do not read past the receive buffer

[Sascha Hauer]

the hardware may report a packet longer than our receive buffer. Instead
of reading past the read buffer, discard too long packets.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 drivers/net/cs8900.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/drivers/net/cs8900.c b/drivers/net/cs8900.c
index afb0f3e26e..a96b574f95 100644
--- a/drivers/net/cs8900.c
+++ b/drivers/net/cs8900.c
@@ -295,8 +295,13 @@ static int cs8900_recv(struct eth_device *dev)
 	status = readw(priv->regs + CS8900_RTDATA0);
 	len = readw(priv->regs + CS8900_RTDATA0);
 
-	for (addr = (u16 *)priv->rx_buf, i = (len + 1) >> 1; i > 0; i--)
-		*addr++ = readw(priv->regs + CS8900_RTDATA0);
+	if (len <= PKTSIZE) {
+		for (addr = (u16 *)priv->rx_buf, i = (len + 1) >> 1; i > 0; i--)
+			*addr++ = readw(priv->regs + CS8900_RTDATA0);
+	} else {
+		for (addr = (u16 *)priv->rx_buf, i = (len + 1) >> 1; i > 0; i--)
+			(void)readw(priv->regs + CS8900_RTDATA0);
+	}
 
 	net_receive(dev, priv->rx_buf, len);
 
-- 
2.39.2

[PATCH 3/5] net: ks8851_mll: do not read past the receive buffer

[Sascha Hauer]

the hardware may report a packet longer than our receive buffer. Instead
of reading past the receive buffer, discard too long packets.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 drivers/net/ks8851_mll.c | 28 +++++++++++++++++++++++++---
 1 file changed, 25 insertions(+), 3 deletions(-)

diff --git a/drivers/net/ks8851_mll.c b/drivers/net/ks8851_mll.c
index 2120657bd9..5e41888b28 100644
--- a/drivers/net/ks8851_mll.c
+++ b/drivers/net/ks8851_mll.c
@@ -420,6 +420,23 @@ static inline void ks_inblk(struct ks_net *ks, u16 *wptr, u32 len)
 		*wptr++ = (u16)readw(ks->hw_addr);
 }
 
+/**
+ * ks_discardblk - read a block of data from QMU discarding the data
+ * @ks: The chip state
+ * @len: length in byte to read
+ *
+ * This discards a block of data, used when a packet is longer than our receive
+ * buffer. I don't know if it is necessary to discard the data like this, but
+ * it is the easy way out to fix the behaviour with too large packets without
+ * risking regressions.
+ */
+static inline void ks_discardblk(struct ks_net *ks, u32 len)
+{
+	len >>= 1;
+	while (len--)
+		(void)readw(ks->hw_addr);
+}
+
 /**
  * ks_outblk - write data to QMU.
  * @ks: The chip information
@@ -676,9 +693,14 @@ static int ks8851_rx_frame(struct ks_net *ks)
 
 	tmp_rxqcr = ks_rdreg16(ks, KS_RXQCR);
 	ks_wrreg16(ks, KS_RXQCR, tmp_rxqcr | RXQCR_SDA);
-	/* read 2 bytes for dummy, 2 for status, 2 for len*/
-	ks_inblk(ks, ks->rx_buf, 2 + 2 + 2);
-	ks_inblk(ks, ks->rx_buf, ALIGN(RxLen, 4));
+	if (RxLen <= PKTSIZE) {
+		/* read 2 bytes for dummy, 2 for status, 2 for len*/
+		ks_inblk(ks, ks->rx_buf, 2 + 2 + 2);
+		ks_inblk(ks, ks->rx_buf, ALIGN(RxLen, 4));
+	} else {
+		ks_discardblk(ks, 2 + 2 + 2 + ALIGN(RxLen, 4));
+	}
+
 	ks_wrreg16(ks, KS_RXQCR, tmp_rxqcr);
 
 	if (RxStatus & RXFSHR_RXFV) {
-- 
2.39.2

[PATCH 4/5] net: liteeth: do not read past the receive buffer

[Sascha Hauer]

The driver already discards packets bigger than 2048 bytes, but that is
already larger than the buffer we read the data into. Limit packet size
to PKTSIZE instead which matches our receive buffer size.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 drivers/net/liteeth.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/liteeth.c b/drivers/net/liteeth.c
index 1781e26348..0d63e1da16 100644
--- a/drivers/net/liteeth.c
+++ b/drivers/net/liteeth.c
@@ -223,7 +223,7 @@ static int liteeth_eth_rx(struct eth_device *edev)
 	}
 
 	len = litex_read32(priv->base + LITEETH_WRITER_LENGTH);
-	if (len == 0 || len > 2048) {
+	if (len == 0 || len > PKTSIZE) {
 		len = 0;
 		dev_err(priv->dev, "%s: invalid len %d\n", __func__, len);
 		litex_write8(priv->base + LITEETH_WRITER_EV_PENDING, reg);
-- 
2.39.2

[PATCH 5/5] net: smc911x: do not read past the receive buffer

[Sascha Hauer]

The hardware may report a packet longer than our receive buffer. Instead
of reading past the read buffer, discard too long packets.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 drivers/net/smc911x.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/drivers/net/smc911x.c b/drivers/net/smc911x.c
index 767d51761b..19117c0af2 100644
--- a/drivers/net/smc911x.c
+++ b/drivers/net/smc911x.c
@@ -460,8 +460,13 @@ static int smc911x_eth_rx(struct eth_device *edev)
 		smc911x_reg_write(priv, RX_CFG, 0);
 
 		tmplen = (pktlen + 2 + 3) / 4;
-		while(tmplen--)
-			*data++ = smc911x_reg_read(priv, RX_DATA_FIFO);
+		if (tmplen <= PKTSIZE / sizeof(u32)) {
+			while (tmplen--)
+				*data++ = smc911x_reg_read(priv, RX_DATA_FIFO);
+		} else {
+			while (tmplen--)
+				smc911x_reg_read(priv, RX_DATA_FIFO);
+		}
 
 		if(status & RX_STS_ES)
 			dev_err(&edev->dev, "dropped bad packet. Status: 0x%08x\n",
-- 
2.39.2

Re: [PATCH 2/5] net: cs8900: do not read past the receive buffer

[Jules Maselbas]

Hi Sascha,

Just a quick remark:

On Mon May 27, 2024 at 9:29 AM CEST, Sascha Hauer wrote:
> the hardware may report a packet longer than our receive buffer. Instead
> of reading past the read buffer, discard too long packets.
>
> Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
> ---
>  drivers/net/cs8900.c | 9 +++++++--
>  1 file changed, 7 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/net/cs8900.c b/drivers/net/cs8900.c
> index afb0f3e26e..a96b574f95 100644
> --- a/drivers/net/cs8900.c
> +++ b/drivers/net/cs8900.c
> @@ -295,8 +295,13 @@ static int cs8900_recv(struct eth_device *dev)
>  	status = readw(priv->regs + CS8900_RTDATA0);
>  	len = readw(priv->regs + CS8900_RTDATA0);
>  
> -	for (addr = (u16 *)priv->rx_buf, i = (len + 1) >> 1; i > 0; i--)
> -		*addr++ = readw(priv->regs + CS8900_RTDATA0);
> +	if (len <= PKTSIZE) {
> +		for (addr = (u16 *)priv->rx_buf, i = (len + 1) >> 1; i > 0; i--)
> +			*addr++ = readw(priv->regs + CS8900_RTDATA0);
> +	} else {
> +		for (addr = (u16 *)priv->rx_buf, i = (len + 1) >> 1; i > 0; i--)
> +			(void)readw(priv->regs + CS8900_RTDATA0);
So the packet is "discarded" here but the function doesn't returns with an error
and proceed to call net_received with the previous (if any) packet but with the
new length ...

I am not sure if this is an issue or not.

> +	}
>  
>  	net_receive(dev, priv->rx_buf, len);
>  


best,
Jules

Re: [PATCH 2/5] net: cs8900: do not read past the receive buffer

[Sascha Hauer]

Hi Jules,

On Mon, May 27, 2024 at 10:58:55AM +0200, Jules Maselbas wrote:
> Hi Sascha,
> 
> Just a quick remark:
> 
> On Mon May 27, 2024 at 9:29 AM CEST, Sascha Hauer wrote:
> > the hardware may report a packet longer than our receive buffer. Instead
> > of reading past the read buffer, discard too long packets.
> >
> > Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
> > ---
> >  drivers/net/cs8900.c | 9 +++++++--
> >  1 file changed, 7 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/net/cs8900.c b/drivers/net/cs8900.c
> > index afb0f3e26e..a96b574f95 100644
> > --- a/drivers/net/cs8900.c
> > +++ b/drivers/net/cs8900.c
> > @@ -295,8 +295,13 @@ static int cs8900_recv(struct eth_device *dev)
> >  	status = readw(priv->regs + CS8900_RTDATA0);
> >  	len = readw(priv->regs + CS8900_RTDATA0);
> >  
> > -	for (addr = (u16 *)priv->rx_buf, i = (len + 1) >> 1; i > 0; i--)
> > -		*addr++ = readw(priv->regs + CS8900_RTDATA0);
> > +	if (len <= PKTSIZE) {
> > +		for (addr = (u16 *)priv->rx_buf, i = (len + 1) >> 1; i > 0; i--)
> > +			*addr++ = readw(priv->regs + CS8900_RTDATA0);
> > +	} else {
> > +		for (addr = (u16 *)priv->rx_buf, i = (len + 1) >> 1; i > 0; i--)
> > +			(void)readw(priv->regs + CS8900_RTDATA0);
> So the packet is "discarded" here but the function doesn't returns with an error
> and proceed to call net_received with the previous (if any) packet but with the
> new length ...

Ouch, that was not intenden. Will fix.

Sascha

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |