linux-kernel - Re: [PATCH] scsi: remove memset before memcpy

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] [day] [month] [year] [list]

Message-ID: <20170829195955.GA26858@himanshu-Vostro-3559>
Date:   Wed, 30 Aug 2017 01:29:55 +0530
From:   Himanshu Jha <himanshujha199640@...il.com>
To:     Joe Perches <joe@...ches.com>
Cc:     jejb@...ux.vnet.ibm.com, martin.petersen@...cle.com,
        kashyap.desai@...adcom.com, anil.gurumurthy@...gic.com,
        sudarsana.kalluru@...gic.com, sumit.saxena@...adcom.com,
        QLogic-Storage-Upstream@...gic.com, linux-scsi@...r.kernel.org,
        linux-kernel@...r.kernel.org, megaraidlinux.pdl@...adcom.com
Subject: Re: [PATCH] scsi: remove memset before memcpy

On Tue, Aug 29, 2017 at 12:29:35PM -0700, Joe Perches wrote:
> On Wed, 2017-08-30 at 00:19 +0530, Himanshu Jha wrote:
> > drivers/scsi/megaraid/megaraid_sas_fusion.c
> 
> I don't know if you did this with coccinelle.

Yes, I did this with coccinelle.

> 
> If so, it would be good to show the tool and script
> in the commit message.

Alright, next time.
> 
> If not, the input script is pretty simple.
> 
> $ cat memset_before_memcpy.cocci
> @@
> expression e1;
> expression e2;
> expression e3;
> @@
> 
> -	memset(e1, 0, e3);
> 	memcpy(e1, e2, e3);
> $
> 
> Adding a test to make sure e1 or e3 isn't
> modified before any other code uses them
> by doing
> 
> $ cat memset_before_memcpy_2.cocci
> @@
> expression e1;
> expression e2;
> expressi
> on e3;
> @@
> 
> -	memset(e1, 0, e3);
> 	... when != \( e1 \| e3 \)
> 	memcpy(e1, e2, e3);
> $
> 
> finds more cases but there may be a
> false positive if e1 is a passed
> function argument and if the operation
> isn't effectively atomic like below:
>

I usually check all my diff before sending to see if any false positives
occured or not.

It's been 4 days since I learnt coccinelle and before I used to look
into every files for occurences or search the identifiers on
FreeElectrons.

>From now I will mention whatever tool I used on my patch!

Thanks

> ----------------------------------------------------------------
> --- a/net/bridge/br_multicast.c
> +++ b/net/bridge/br_multicast.c
> @@ -2556,7 +2556,6 @@ void br_multicast_get_stats(const struct
>  	struct br_mcast_stats tdst;
>  	int i;
>  
> -	memset(dest, 0, sizeof(*dest));
>  	if (p)
>  		stats = p->mcast_stats;
>  	else
> 
> ----------------------------------------------------------------
> 
> where the memcpy is the last line of the function.
>