netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 1/2] net: encx24j600: Add parentheses to fix precedence
@ 2022-12-01 17:34 Valentina Goncharenko
  2022-12-01 17:34 ` [PATCH 2/2] net: encx24j600: Fix invalid logic in reading of MISTAT register Valentina Goncharenko
                   ` (2 more replies)
  0 siblings, 3 replies; 5+ messages in thread
From: Valentina Goncharenko @ 2022-12-01 17:34 UTC (permalink / raw)
  To: David S. Miller
  Cc: Valentina Goncharenko, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
	Jon Ringle, netdev, linux-kernel, lvc-project

In functions regmap_encx24j600_phy_reg_read() and
regmap_encx24j600_phy_reg_write() in the conditions of the waiting
cycles for filling the variable 'ret' it is necessary to add parentheses
to prevent wrong assignment due to logical operations precedence.

Found by Linux Verification Center (linuxtesting.org) with SVACE.

Fixes: d70e53262f5c ("net: Microchip encx24j600 driver")
Signed-off-by: Valentina Goncharenko <goncharenko.vp@ispras.ru>
---
 drivers/net/ethernet/microchip/encx24j600-regmap.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/microchip/encx24j600-regmap.c b/drivers/net/ethernet/microchip/encx24j600-regmap.c
index 81a8ccca7e5e..2e337c7a5773 100644
--- a/drivers/net/ethernet/microchip/encx24j600-regmap.c
+++ b/drivers/net/ethernet/microchip/encx24j600-regmap.c
@@ -359,7 +359,7 @@ static int regmap_encx24j600_phy_reg_read(void *context, unsigned int reg,
 		goto err_out;
 
 	usleep_range(26, 100);
-	while ((ret = regmap_read(ctx->regmap, MISTAT, &mistat) != 0) &&
+	while (((ret = regmap_read(ctx->regmap, MISTAT, &mistat)) != 0) &&
 	       (mistat & BUSY))
 		cpu_relax();
 
@@ -397,7 +397,7 @@ static int regmap_encx24j600_phy_reg_write(void *context, unsigned int reg,
 		goto err_out;
 
 	usleep_range(26, 100);
-	while ((ret = regmap_read(ctx->regmap, MISTAT, &mistat) != 0) &&
+	while (((ret = regmap_read(ctx->regmap, MISTAT, &mistat)) != 0) &&
 	       (mistat & BUSY))
 		cpu_relax();
 
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* [PATCH 2/2] net: encx24j600: Fix invalid logic in reading of MISTAT register
  2022-12-01 17:34 [PATCH 1/2] net: encx24j600: Add parentheses to fix precedence Valentina Goncharenko
@ 2022-12-01 17:34 ` Valentina Goncharenko
  2022-12-02  7:55   ` Pavan Chebbi
  2022-12-02  7:50 ` [PATCH 1/2] net: encx24j600: Add parentheses to fix precedence Pavan Chebbi
  2022-12-05  9:50 ` patchwork-bot+netdevbpf
  2 siblings, 1 reply; 5+ messages in thread
From: Valentina Goncharenko @ 2022-12-01 17:34 UTC (permalink / raw)
  To: David S. Miller
  Cc: Valentina Goncharenko, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
	Jon Ringle, netdev, linux-kernel, lvc-project

A loop for reading MISTAT register continues while regmap_read() fails
and (mistat & BUSY), but if regmap_read() fails a value of mistat is
undefined.

The patch proposes to check for BUSY flag only when regmap_read()
succeed. Compile test only.

Found by Linux Verification Center (linuxtesting.org) with SVACE.

Fixes: d70e53262f5c ("net: Microchip encx24j600 driver")
Signed-off-by: Valentina Goncharenko <goncharenko.vp@ispras.ru>
---
 drivers/net/ethernet/microchip/encx24j600-regmap.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/microchip/encx24j600-regmap.c b/drivers/net/ethernet/microchip/encx24j600-regmap.c
index 2e337c7a5773..5693784eec5b 100644
--- a/drivers/net/ethernet/microchip/encx24j600-regmap.c
+++ b/drivers/net/ethernet/microchip/encx24j600-regmap.c
@@ -359,7 +359,7 @@ static int regmap_encx24j600_phy_reg_read(void *context, unsigned int reg,
 		goto err_out;
 
 	usleep_range(26, 100);
-	while (((ret = regmap_read(ctx->regmap, MISTAT, &mistat)) != 0) &&
+	while (((ret = regmap_read(ctx->regmap, MISTAT, &mistat)) == 0) &&
 	       (mistat & BUSY))
 		cpu_relax();
 
@@ -397,7 +397,7 @@ static int regmap_encx24j600_phy_reg_write(void *context, unsigned int reg,
 		goto err_out;
 
 	usleep_range(26, 100);
-	while (((ret = regmap_read(ctx->regmap, MISTAT, &mistat)) != 0) &&
+	while (((ret = regmap_read(ctx->regmap, MISTAT, &mistat)) == 0) &&
 	       (mistat & BUSY))
 		cpu_relax();
 
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH 1/2] net: encx24j600: Add parentheses to fix precedence
  2022-12-01 17:34 [PATCH 1/2] net: encx24j600: Add parentheses to fix precedence Valentina Goncharenko
  2022-12-01 17:34 ` [PATCH 2/2] net: encx24j600: Fix invalid logic in reading of MISTAT register Valentina Goncharenko
@ 2022-12-02  7:50 ` Pavan Chebbi
  2022-12-05  9:50 ` patchwork-bot+netdevbpf
  2 siblings, 0 replies; 5+ messages in thread
From: Pavan Chebbi @ 2022-12-02  7:50 UTC (permalink / raw)
  To: Valentina Goncharenko
  Cc: David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
	Jon Ringle, netdev, linux-kernel, lvc-project

[-- Attachment #1: Type: text/plain, Size: 1808 bytes --]

On Thu, Dec 1, 2022 at 11:04 PM Valentina Goncharenko
<goncharenko.vp@ispras.ru> wrote:
>
> In functions regmap_encx24j600_phy_reg_read() and
> regmap_encx24j600_phy_reg_write() in the conditions of the waiting
> cycles for filling the variable 'ret' it is necessary to add parentheses
> to prevent wrong assignment due to logical operations precedence.
>
> Found by Linux Verification Center (linuxtesting.org) with SVACE.
>
> Fixes: d70e53262f5c ("net: Microchip encx24j600 driver")
> Signed-off-by: Valentina Goncharenko <goncharenko.vp@ispras.ru>
> ---
>  drivers/net/ethernet/microchip/encx24j600-regmap.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/net/ethernet/microchip/encx24j600-regmap.c b/drivers/net/ethernet/microchip/encx24j600-regmap.c
> index 81a8ccca7e5e..2e337c7a5773 100644
> --- a/drivers/net/ethernet/microchip/encx24j600-regmap.c
> +++ b/drivers/net/ethernet/microchip/encx24j600-regmap.c
> @@ -359,7 +359,7 @@ static int regmap_encx24j600_phy_reg_read(void *context, unsigned int reg,
>                 goto err_out;
>
>         usleep_range(26, 100);
> -       while ((ret = regmap_read(ctx->regmap, MISTAT, &mistat) != 0) &&
> +       while (((ret = regmap_read(ctx->regmap, MISTAT, &mistat)) != 0) &&
>                (mistat & BUSY))
>                 cpu_relax();
>
> @@ -397,7 +397,7 @@ static int regmap_encx24j600_phy_reg_write(void *context, unsigned int reg,
>                 goto err_out;
>
>         usleep_range(26, 100);
> -       while ((ret = regmap_read(ctx->regmap, MISTAT, &mistat) != 0) &&
> +       while (((ret = regmap_read(ctx->regmap, MISTAT, &mistat)) != 0) &&
>                (mistat & BUSY))
>                 cpu_relax();
>
> --
> 2.25.1
>
Makes sense to me.
Reviewed-by: Pavan Chebbi <pavan.chebbi@broadcom.com>

[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4209 bytes --]

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH 2/2] net: encx24j600: Fix invalid logic in reading of MISTAT register
  2022-12-01 17:34 ` [PATCH 2/2] net: encx24j600: Fix invalid logic in reading of MISTAT register Valentina Goncharenko
@ 2022-12-02  7:55   ` Pavan Chebbi
  0 siblings, 0 replies; 5+ messages in thread
From: Pavan Chebbi @ 2022-12-02  7:55 UTC (permalink / raw)
  To: Valentina Goncharenko
  Cc: David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
	Jon Ringle, netdev, linux-kernel, lvc-project

[-- Attachment #1: Type: text/plain, Size: 1802 bytes --]

On Thu, Dec 1, 2022 at 11:04 PM Valentina Goncharenko
<goncharenko.vp@ispras.ru> wrote:
>
> A loop for reading MISTAT register continues while regmap_read() fails
> and (mistat & BUSY), but if regmap_read() fails a value of mistat is
> undefined.
>
> The patch proposes to check for BUSY flag only when regmap_read()
> succeed. Compile test only.
>
> Found by Linux Verification Center (linuxtesting.org) with SVACE.
>
> Fixes: d70e53262f5c ("net: Microchip encx24j600 driver")
> Signed-off-by: Valentina Goncharenko <goncharenko.vp@ispras.ru>
> ---
>  drivers/net/ethernet/microchip/encx24j600-regmap.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/net/ethernet/microchip/encx24j600-regmap.c b/drivers/net/ethernet/microchip/encx24j600-regmap.c
> index 2e337c7a5773..5693784eec5b 100644
> --- a/drivers/net/ethernet/microchip/encx24j600-regmap.c
> +++ b/drivers/net/ethernet/microchip/encx24j600-regmap.c
> @@ -359,7 +359,7 @@ static int regmap_encx24j600_phy_reg_read(void *context, unsigned int reg,
>                 goto err_out;
>
>         usleep_range(26, 100);
> -       while (((ret = regmap_read(ctx->regmap, MISTAT, &mistat)) != 0) &&
> +       while (((ret = regmap_read(ctx->regmap, MISTAT, &mistat)) == 0) &&
>                (mistat & BUSY))
>                 cpu_relax();
>
> @@ -397,7 +397,7 @@ static int regmap_encx24j600_phy_reg_write(void *context, unsigned int reg,
>                 goto err_out;
>
>         usleep_range(26, 100);
> -       while (((ret = regmap_read(ctx->regmap, MISTAT, &mistat)) != 0) &&
> +       while (((ret = regmap_read(ctx->regmap, MISTAT, &mistat)) == 0) &&
>                (mistat & BUSY))
>                 cpu_relax();
>
> --
> 2.25.1
>
Looks OK to me.
Reviewed-by: Pavan Chebbi <pavan.chebbi@broadcom.com>

[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4209 bytes --]

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH 1/2] net: encx24j600: Add parentheses to fix precedence
  2022-12-01 17:34 [PATCH 1/2] net: encx24j600: Add parentheses to fix precedence Valentina Goncharenko
  2022-12-01 17:34 ` [PATCH 2/2] net: encx24j600: Fix invalid logic in reading of MISTAT register Valentina Goncharenko
  2022-12-02  7:50 ` [PATCH 1/2] net: encx24j600: Add parentheses to fix precedence Pavan Chebbi
@ 2022-12-05  9:50 ` patchwork-bot+netdevbpf
  2 siblings, 0 replies; 5+ messages in thread
From: patchwork-bot+netdevbpf @ 2022-12-05  9:50 UTC (permalink / raw)
  To: Valentina Goncharenko
  Cc: davem, edumazet, kuba, pabeni, jringle, netdev, linux-kernel,
	lvc-project

Hello:

This series was applied to netdev/net.git (master)
by David S. Miller <davem@davemloft.net>:

On Thu,  1 Dec 2022 20:34:07 +0300 you wrote:
> In functions regmap_encx24j600_phy_reg_read() and
> regmap_encx24j600_phy_reg_write() in the conditions of the waiting
> cycles for filling the variable 'ret' it is necessary to add parentheses
> to prevent wrong assignment due to logical operations precedence.
> 
> Found by Linux Verification Center (linuxtesting.org) with SVACE.
> 
> [...]

Here is the summary with links:
  - [1/2] net: encx24j600: Add parentheses to fix precedence
    https://git.kernel.org/netdev/net/c/167b3f2dcc62
  - [2/2] net: encx24j600: Fix invalid logic in reading of MISTAT register
    https://git.kernel.org/netdev/net/c/25f427ac7b8d

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html



^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2022-12-05  9:50 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-12-01 17:34 [PATCH 1/2] net: encx24j600: Add parentheses to fix precedence Valentina Goncharenko
2022-12-01 17:34 ` [PATCH 2/2] net: encx24j600: Fix invalid logic in reading of MISTAT register Valentina Goncharenko
2022-12-02  7:55   ` Pavan Chebbi
2022-12-02  7:50 ` [PATCH 1/2] net: encx24j600: Add parentheses to fix precedence Pavan Chebbi
2022-12-05  9:50 ` patchwork-bot+netdevbpf

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).