linux-kernel - Re: [PATCH] arch/powerpc: Remove unnecessary endian conversion code in XICS

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <878rbxwl7e.fsf@mail.lhotse>
Date:   Mon, 03 Jul 2023 16:41:41 +1000
From:   Michael Ellerman <mpe@...erman.id.au>
To:     Gautam Menghani <gautam@...ux.ibm.com>, npiggin@...il.com,
        christophe.leroy@...roup.eu
Cc:     Gautam Menghani <gautam@...ux.ibm.com>,
        linuxppc-dev@...ts.ozlabs.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] arch/powerpc: Remove unnecessary endian conversion code
 in XICS

Gautam Menghani <gautam@...ux.ibm.com> writes:
> Remove an unnecessary piece of code that does an endianness conversion but
> does not use the result. The following warning was reported by Clang's
> static analyzer:
>
> arch/powerpc/sysdev/xics/ics-opal.c:114:2: warning: Value stored to
> 'server' is never read [deadcode.DeadStores]
>         server = be16_to_cpu(oserver);
>
> As the result of endianness conversion is never used, delete the line
> and fix the warning.
>
> Signed-off-by: Gautam Menghani <gautam@...ux.ibm.com>
> ---
>  arch/powerpc/sysdev/xics/ics-opal.c | 1 -
>  1 file changed, 1 deletion(-)
>
> diff --git a/arch/powerpc/sysdev/xics/ics-opal.c b/arch/powerpc/sysdev/xics/ics-opal.c
> index 6cfbb4fac7fb..5fe73dabab79 100644
> --- a/arch/powerpc/sysdev/xics/ics-opal.c
> +++ b/arch/powerpc/sysdev/xics/ics-opal.c
> @@ -111,7 +111,6 @@ static int ics_opal_set_affinity(struct irq_data *d,
>  		       __func__, d->irq, hw_irq, rc);
>  		return -1;
>  	}
> -	server = be16_to_cpu(oserver);
>  
>  	wanted_server = xics_get_irq_server(d->irq, cpumask, 1);
>  	if (wanted_server < 0) {

My first question with a patch like this is always going to be "how did
the code end up like this?"

Has the code changed and this assignment became unused? If so the commit
that did that should be identified.

If the code has always been like this that's also useful to know. Or
something else happened for it to end up this way :)

The second question will be "is there actually a bug here?". ie. should
server actually be used, and the bug is not that it's a dead assignment
but rather that server is not where it should be.

cheers