linux-hardening - replacing memcpy() calls with direct assignment

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

Date:   Tue, 21 Jun 2022 11:37:10 -0700
From:   Kees Cook <keescook@...omium.org>
To:     Coccinelle <cocci@...teme.lip6.fr>
Cc:     linux-hardening@...r.kernel.org,
        Julia Lawall <Julia.Lawall@...ia.fr>
Subject: replacing memcpy() calls with direct assignment

Hello Coccinelle gurus! :)

I recently spent way too long looking at a weird bug in Clang that I
eventually worked around by just replacing a memcpy() with a direct
assignment. It really was very mechanical, and seems like it might be a
common code pattern in the kernel. Swapping these would make the code
much more readable, I think. Here's the example:


https://lore.kernel.org/linux-hardening/20220616052312.292861-1-keescook@chromium.org/

-		memcpy(&host_image->image_section_info[i],
-		       &fw_image->fw_section_info[i],
-		       sizeof(struct fw_section_info_st));
+		host_image->image_section_info[i] = fw_image->fw_section_info[i];

Is there a way to reduce the size of this cocci rule? I had to
explicitly spell out each "address of" condition separately, though I'd
expect them to be internal aliases, but I'd get output like:

 *&dst = src;

etc

@direct_assignment@
type TYPE;
TYPE DST, SRC;
TYPE *DPTR;
TYPE *SPTR;
@@

(
- memcpy(&DST, &SRC, sizeof(TYPE))
+ DST = SRC
|
- memcpy(&DST, &SRC, sizeof(DST))
+ DST = SRC
|
- memcpy(&DST, &SRC, sizeof(SRC))
+ DST = SRC
|

- memcpy(&DST, SPTR, sizeof(TYPE))
+ DST = *SPTR
|
- memcpy(&DST, SPTR, sizeof(DST))
+ DST = *SPTR
|
- memcpy(&DST, SPTR, sizeof(*SPTR))
+ DST = *SPTR
|

- memcpy(DPTR, &SRC, sizeof(TYPE))
+ *DPTR = SRC
|
- memcpy(DPTR, &SRC, sizeof(DST))
+ *DPTR = SRC
|
- memcpy(DPTR, &SRC, sizeof(SRC))
+ *DPTR = SRC
|

- memcpy(DPTR, SPTR, sizeof(TYPE))
+ *DPTR = *SPTR
|
- memcpy(DPTR, SPTR, sizeof(*DST))
+ *DPTR = *SPTR
|
- memcpy(DPTR, SPTR, sizeof(*SRC))
+ *DPTR = *SPTR
)

-- 
Kees Cook