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]
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
> 

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ