2009-12-27 21:19:19

by Roel Kluin

Subject: [PATCH] usbnet: test off by one

With `while (i++ < MII_TIMEOUT)' i reaches MII_TIMEOUT + 1 after the loop
This is probably unlikely a problem in practice.

Signed-off-by: Roel Kluin <[email protected]>
---
drivers/net/usb/rtl8150.c | 4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/usb/rtl8150.c b/drivers/net/usb/rtl8150.c
index f14d225..fd19db0 100644
--- a/drivers/net/usb/rtl8150.c
+++ b/drivers/net/usb/rtl8150.c
@@ -270,7 +270,7 @@ static int read_mii_word(rtl8150_t * dev, u8 phy, __u8 indx, u16 * reg)
get_registers(dev, PHYCNT, 1, data);
} while ((data[0] & PHY_GO) && (i++ < MII_TIMEOUT));

- if (i < MII_TIMEOUT) {
+ if (i <= MII_TIMEOUT) {
get_registers(dev, PHYDAT, 2, data);
*reg = data[0] | (data[1] << 8);
return 0;
@@ -295,7 +295,7 @@ static int write_mii_word(rtl8150_t * dev, u8 phy, __u8 indx, u16 reg)
get_registers(dev, PHYCNT, 1, data);
} while ((data[0] & PHY_GO) && (i++ < MII_TIMEOUT));

- if (i < MII_TIMEOUT)
+ if (i <= MII_TIMEOUT)
return 0;
else
return 1;

2009-12-29 13:49:08

by Petko Manolov

Subject: Re: [PATCH] usbnet: test off by one

Hey Roel,

MII_TIMEOUT is a number i made up myself, reasonably small so the driver
won't sit there forever, but higher than 2. While testing the gadget i
discovered that reading MII registers sometimes require more than one
attempt.

Technically you're right. In practice it doesn't matter much. Hitting
the boundary condition mean things are already out of hand.


Petko



On Sun, 27 Dec 2009, Roel Kluin wrote:

> With `while (i++ < MII_TIMEOUT)' i reaches MII_TIMEOUT + 1 after the loop
> This is probably unlikely a problem in practice.
>
> Signed-off-by: Roel Kluin <[email protected]>
> ---
> drivers/net/usb/rtl8150.c | 4 ++--
> 1 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/net/usb/rtl8150.c b/drivers/net/usb/rtl8150.c
> index f14d225..fd19db0 100644
> --- a/drivers/net/usb/rtl8150.c
> +++ b/drivers/net/usb/rtl8150.c
> @@ -270,7 +270,7 @@ static int read_mii_word(rtl8150_t * dev, u8 phy, __u8 indx, u16 * reg)
> get_registers(dev, PHYCNT, 1, data);
> } while ((data[0] & PHY_GO) && (i++ < MII_TIMEOUT));
>
> - if (i < MII_TIMEOUT) {
> + if (i <= MII_TIMEOUT) {
> get_registers(dev, PHYDAT, 2, data);
> *reg = data[0] | (data[1] << 8);
> return 0;
> @@ -295,7 +295,7 @@ static int write_mii_word(rtl8150_t * dev, u8 phy, __u8 indx, u16 reg)
> get_registers(dev, PHYCNT, 1, data);
> } while ((data[0] & PHY_GO) && (i++ < MII_TIMEOUT));
>
> - if (i < MII_TIMEOUT)
> + if (i <= MII_TIMEOUT)
> return 0;
> else
> return 1;
>

2010-01-04 05:43:43

by David Miller

Subject: Re: [PATCH] usbnet: test off by one

From: Roel Kluin <[email protected]>
Date: Sun, 27 Dec 2009 22:22:08 +0100

> With `while (i++ < MII_TIMEOUT)' i reaches MII_TIMEOUT + 1 after the loop
> This is probably unlikely a problem in practice.
>
> Signed-off-by: Roel Kluin <[email protected]>

Applied.