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

Message-ID: <1504034975.2040.43.camel@perches.com>
Date:   Tue, 29 Aug 2017 12:29:35 -0700
From:   Joe Perches <joe@...ches.com>
To:     Himanshu Jha <himanshujha199640@...il.com>, jejb@...ux.vnet.ibm.com
Cc:     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 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.

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

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:

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