linux-kernel - Re: [PATCH] ipc/sem: Remove redundant assignments

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 for Android: free password hash cracker in your pocket

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

Message-ID: <YlRNmXucZwNasoFq@thelio-3990X>
Date:   Mon, 11 Apr 2022 08:47:37 -0700
From:   Nathan Chancellor <nathan@...nel.org>
To:     Michal Orzel <michalorzel.eng@...il.com>
Cc:     Nick Desaulniers <ndesaulniers@...gle.com>,
        Tom Rix <trix@...hat.com>, linux-kernel@...r.kernel.org,
        llvm@...ts.linux.dev, Andrew Morton <akpm@...ux-foundation.org>
Subject: Re: [PATCH] ipc/sem: Remove redundant assignments

Hi Michal,

On Sat, Apr 09, 2022 at 12:19:31PM +0200, Michal Orzel wrote:
> Get rid of redundant assignments which end up in values not being
> read either because they are overwritten or the function ends.
> 
> Reported by clang-tidy [deadcode.DeadStores]
> 
> Signed-off-by: Michal Orzel <michalorzel.eng@...il.com>

Thank you for the patch! This should go through Andrew Morton, now CC'd.

For the future, having some history behind the origin of these warnings
would make review a little easier. I'll comment on each below.

Reviewed-by: Nathan Chancellor <nathan@...nel.org>

> ---
>  ipc/sem.c | 2 --
>  1 file changed, 2 deletions(-)
> 
> diff --git a/ipc/sem.c b/ipc/sem.c
> index 0dbdb98fdf2d..38ef91a63edd 100644
> --- a/ipc/sem.c
> +++ b/ipc/sem.c
> @@ -766,7 +766,6 @@ static int perform_atomic_semop(struct sem_array *sma, struct sem_queue *q)
>  	for (sop = sops; sop < sops + nsops; sop++) {
>  		curr = &sma->sems[sop->sem_num];
>  		sem_op = sop->sem_op;
> -		result = curr->semval;

Caused by commit 4ce33ec2e42d ("ipc/sem: optimize
perform_atomic_semop()"). Seems like a simple copy and paste bug.

>  		if (sop->sem_flg & SEM_UNDO) {
>  			int undo = un->semadj[sop->sem_num] - sem_op;
> @@ -1430,7 +1429,6 @@ static int semctl_main(struct ipc_namespace *ns, int semid, int semnum,
>  	if (err)
>  		goto out_rcu_wakeup;
>  
> -	err = -EACCES;

I was not able to figure out this one but it does look correct, as err
is either set properly in the switch statement below or we skip over it
to the 'err = -EINVAL'.

>  	switch (cmd) {
>  	case GETALL:
>  	{
> -- 
> 2.25.1
>