[PATCH 5.10 0/1] usb: musb: core: drop redundant checks

[PATCH 5.10 0/1] usb: musb: core: drop redundant checks

[Danila Chernetsov]

hw_ep can't be NULL but compared to NULL in 5.10 stable releases. 
The problem has been fixed by the following 
patch which can be cleanly applied to the 5.10 branch. 

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

[PATCH 5.10 1/1] usb: musb: core: drop redundant checks

[Danila Chernetsov]

From: Sergey Shtylyov <s.shtylyov@omp.ru>

commit b0ec7e55fce65f125bd1d7f02e2dc4de62abee34 upstream. 

In musb_{save|restore}_context() the expression '&musb->endpoints[i]' just
cannot be NULL, so the checks have no sense at all -- after dropping them,
the local variables 'hw_ep' are no longer necessary, so drop them as well.

Found by Linux Verification Center (linuxtesting.org) with the SVACE static
analysis tool.

Signed-off-by: Sergey Shtylyov <s.shtylyov@omp.ru>
Link: https://lore.kernel.org/r/3f8f60d9-f1b5-6b2c-1222-39b156151a22@omp.ru
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Signed-off-by: Danila Chernetsov <listdansp@mail.ru>
---
 drivers/usb/musb/musb_core.c | 16 ++--------------
 1 file changed, 2 insertions(+), 14 deletions(-)

diff --git a/drivers/usb/musb/musb_core.c b/drivers/usb/musb/musb_core.c
index 4c8f0112481f..605f5cc0f18b 100644
--- a/drivers/usb/musb/musb_core.c
+++ b/drivers/usb/musb/musb_core.c
@@ -2673,13 +2673,7 @@ static void musb_save_context(struct musb *musb)
 	musb->context.devctl = musb_readb(musb_base, MUSB_DEVCTL);
 
 	for (i = 0; i < musb->config->num_eps; ++i) {
-		struct musb_hw_ep	*hw_ep;
-
-		hw_ep = &musb->endpoints[i];
-		if (!hw_ep)
-			continue;
-
-		epio = hw_ep->regs;
+		epio = musb->endpoints[i].regs;
 		if (!epio)
 			continue;
 
@@ -2754,13 +2748,7 @@ static void musb_restore_context(struct musb *musb)
 		musb_writeb(musb_base, MUSB_DEVCTL, musb->context.devctl);
 
 	for (i = 0; i < musb->config->num_eps; ++i) {
-		struct musb_hw_ep	*hw_ep;
-
-		hw_ep = &musb->endpoints[i];
-		if (!hw_ep)
-			continue;
-
-		epio = hw_ep->regs;
+		epio = musb->endpoints[i].regs;
 		if (!epio)
 			continue;
 
-- 
2.25.1

Re: [PATCH 5.10 1/1] usb: musb: core: drop redundant checks

[Sergey Shtylyov]

On 3/14/23 8:01 PM, Danila Chernetsov wrote:

> From: Sergey Shtylyov <s.shtylyov@omp.ru>
> 
> commit b0ec7e55fce65f125bd1d7f02e2dc4de62abee34 upstream. 
> 
> In musb_{save|restore}_context() the expression '&musb->endpoints[i]' just
> cannot be NULL, so the checks have no sense at all -- after dropping them,
> the local variables 'hw_ep' are no longer necessary, so drop them as well.
> 
> Found by Linux Verification Center (linuxtesting.org) with the SVACE static
> analysis tool.
> 
> Signed-off-by: Sergey Shtylyov <s.shtylyov@omp.ru>
> Link: https://lore.kernel.org/r/3f8f60d9-f1b5-6b2c-1222-39b156151a22@omp.ru
> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Signed-off-by: Danila Chernetsov <listdansp@mail.ru>
> ---
>  drivers/usb/musb/musb_core.c | 16 ++--------------
>  1 file changed, 2 insertions(+), 14 deletions(-)

   Hum... why do you need it it 5.10.y?

[...]

MBR, Sergey

Re: [PATCH 5.10 1/1] usb: musb: core: drop redundant checks

[Greg Kroah-Hartman]

On Tue, Mar 14, 2023 at 05:01:13PM +0000, Danila Chernetsov wrote:
> From: Sergey Shtylyov <s.shtylyov@omp.ru>
> 
> commit b0ec7e55fce65f125bd1d7f02e2dc4de62abee34 upstream. 

Nit, trailing whitespace, please fix your editor to show this up easier.

> 
> In musb_{save|restore}_context() the expression '&musb->endpoints[i]' just
> cannot be NULL, so the checks have no sense at all -- after dropping them,
> the local variables 'hw_ep' are no longer necessary, so drop them as well.
> 
> Found by Linux Verification Center (linuxtesting.org) with the SVACE static
> analysis tool.
> 
> Signed-off-by: Sergey Shtylyov <s.shtylyov@omp.ru>
> Link: https://lore.kernel.org/r/3f8f60d9-f1b5-6b2c-1222-39b156151a22@omp.ru
> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Signed-off-by: Danila Chernetsov <listdansp@mail.ru>
> ---
>  drivers/usb/musb/musb_core.c | 16 ++--------------
>  1 file changed, 2 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/usb/musb/musb_core.c b/drivers/usb/musb/musb_core.c
> index 4c8f0112481f..605f5cc0f18b 100644
> --- a/drivers/usb/musb/musb_core.c
> +++ b/drivers/usb/musb/musb_core.c
> @@ -2673,13 +2673,7 @@ static void musb_save_context(struct musb *musb)
>  	musb->context.devctl = musb_readb(musb_base, MUSB_DEVCTL);
>  
>  	for (i = 0; i < musb->config->num_eps; ++i) {
> -		struct musb_hw_ep	*hw_ep;
> -
> -		hw_ep = &musb->endpoints[i];
> -		if (!hw_ep)
> -			continue;
> -
> -		epio = hw_ep->regs;
> +		epio = musb->endpoints[i].regs;

Why is this needed in the stable releases?  It just seems to remove
unused code, but does not change any logic at all, so why backport it?

thanks,

greg k-h

Re: [PATCH 5.10 1/1] usb: musb: core: drop redundant checks

[Greg Kroah-Hartman]

On Wed, Mar 15, 2023 at 07:16:44AM +0300, listdansp wrote:
> This patch was prepare in according to secure programming conception.

I do not understand what that means.

> In practice it indeed simply remove unused code.

Yes, it did, but why should that be added to a stable kernel tree?

> If you're thinking, that this patch is useless, we don't insist on applying
> it.

I'm confused as to why you thought it should have been applied at all.
Why did your testing deem it needed?

thanks,

greg k-h