[LU-16510] Provide an interim 'good' fortified memcpy from 6.1

• Status: Resolved
• Project: Lustre
• Fix Version/s: Lustre 2.16.0, Lustre 2.15.3
• Type: Bug
• Priority: Minor
• Reporter: Shaun Tancheff
• Assignee: Patrick Farrell
• Resolution: Fixed
• Severity: 3

Description

The fortified memcpy() from kernel v5.11-11104-ga28a6e860c6c through v5.18-rc5-1405-g43213daed6d6 incorrectly reports a false positive.

 13:54:25 In function 'memcpy',
 13:54:25     inlined from 'lov_iocontrol' at /tmp/rpmbuild-lustre-jenkins-4fGYbbr1/BUILD/lustre-2.15.53_116_g5d22f15/lustre/lov/lov_obd.c:1057:4:
 13:54:25 include/linux/fortify-string.h:187:25: error: call to '__read_overflow2' declared with attribute error: detected read beyond size of object passed as 2nd parameter
 13:54:25   187 |                         __read_overflow2();
 13:54:25       |                         ^~~~~~~~~~~~~~~~~~

Comments

[Gerrit Updater]

"Shaun Tancheff <[e-mail]>" uploaded a new patch: https://review.whamcloud.com/c/fs/lustre-release/+/49811
Subject: LU-16510 build: fortified memcpy from linux 6.1
Project: fs/lustre-release
Branch: master
Current Patch Set: 1
Commit: e4b87269f249f94c004ffb3d7ed6c4b369956fcf

[Gerrit Updater]

"Jian Yu <[e-mail]>" uploaded a new patch: https://review.whamcloud.com/c/fs/lustre-release/+/49815
Subject: LU-16510 build: fortified memcpy from linux 6.1
Project: fs/lustre-release
Branch: b2_15
Current Patch Set: 1
Commit: 0217d59249441102d6d4d51e6ec8d87d339d5e8c

[Gerrit Updater]

"Oleg Drokin <[e-mail]>" merged in patch https://review.whamcloud.com/c/fs/lustre-release/+/49811/
Subject: LU-16510 build: fortified memcpy from linux 6.1
Project: fs/lustre-release
Branch: master
Current Patch Set:
Commit: 919b93b951d4a9aa0400b9c882a1f68b79d8f118

[Peter Jones]

Landed for 2.16

[Gerrit Updater]

"Oleg Drokin <[e-mail]>" merged in patch https://review.whamcloud.com/c/fs/lustre-release/+/49815/
Subject: LU-16510 build: fortified memcpy from linux 6.1
Project: fs/lustre-release
Branch: b2_15
Current Patch Set:
Commit: 83658e38cd497c777a3056edebc8cd34803b45f8

[Patrick Farrell]

adegremont_nvda pointed out this patch is missing a key component.  Quoting him:

"

LU-16510 has a defect. The fortify.h file is missing one important bit compared to the upstream file it copied:

+/**
+ * unsafe_memcpy - memcpy implementation with no FORTIFY bounds checking
+ *
+ * @dst: Destination memory address to write to
+ * @src: Source memory address to read from
+ * @bytes: How many bytes to write to @dst from @src
+ * @justification: Free-form text or comment describing why the use is needed
+ *
+ * This should be used for corner cases where the compiler cannot do the
+ * right thing, or during transitions between APIs, etc. It should be used
+ * very rarely, and includes a place for justification detailing where bounds
+ * checking has happened, and why existing solutions cannot be employed.
+ */
+#define unsafe_memcpy(dst, src, bytes, justification)          \
+       __underlying_memcpy(dst, src, bytes)
+

"

I'll push a patch.

[Gerrit Updater]

"Patrick Farrell <[e-mail]>" uploaded a new patch: https://review.whamcloud.com/c/fs/lustre-release/+/50573
Subject: LU-16510 build: include unsafe_memcpy definition
Project: fs/lustre-release
Branch: master
Current Patch Set: 1
Commit: 991db242d198ab56d354cd374bf8606fa312fd65

[Gerrit Updater]

"Oleg Drokin <[e-mail]>" merged in patch https://review.whamcloud.com/c/fs/lustre-release/+/50573/
Subject: LU-16510 build: include unsafe_memcpy definition
Project: fs/lustre-release
Branch: master
Current Patch Set:
Commit: 565b21bf65e385a9b4fd8ee31cabe7892345b783

[Gerrit Updater]

"Jian Yu <[e-mail]>" uploaded a new patch: https://review.whamcloud.com/c/fs/lustre-release/+/50667
Subject: LU-16510 build: include unsafe_memcpy definition
Project: fs/lustre-release
Branch: b2_15
Current Patch Set: 1
Commit: 379a6b3d4696e28b1db6a5bfd0d6f92fbe6378de

[Peter Jones]

Landed for 2.16

[Gerrit Updater]

"Oleg Drokin <[e-mail]>" merged in patch https://review.whamcloud.com/c/fs/lustre-release/+/50667/
Subject: LU-16510 build: include unsafe_memcpy definition
Project: fs/lustre-release
Branch: b2_15
Current Patch Set:
Commit: 8f4004809461bb2cab20c72d20980edf32c98793

[Gerrit Updater]

"Jian Yu <[e-mail]>" uploaded a new patch: https://review.whamcloud.com/c/fs/lustre-release/+/51973
Subject: LU-16510 build: check if CONFIG_FORTIFY_SOURCE is defined
Project: fs/lustre-release
Branch: master
Current Patch Set: 1
Commit: 06956ef63fd7a0489648fe85c2f1e1904e78424e

[Gerrit Updater]

"Oleg Drokin <[e-mail]>" merged in patch https://review.whamcloud.com/c/fs/lustre-release/+/51973/
Subject: LU-16510 build: check if CONFIG_FORTIFY_SOURCE is defined
Project: fs/lustre-release
Branch: master
Current Patch Set:
Commit: a52545afeda06441841f6143d8fd563251549acb

[James A Simmons]

Another bug has shown up. This time its the __QCTL_COPY macro in lustre_idl.h that fails to build.
In function 'memcpy',10:33:49 inlined from 'lov_iocontrol' at /tmp/rpmbuild-lustre-jenkins-MOxpnLlK/BUILD/lustre-2.15.58_1_g13e68ce/lustre/lov/lov_obd.c:1079:4:10:33:49 include/linux/fortify-string.h:187:25: error: call to '__read_overflow2' declared with attribute error: detected read beyond size of object passed as 2nd parameter*10:33:49* 187 | __read_overflow2();10:33:49 | ^~~~~~~~~~~~~~~~~~

[James A Simmons]

I looked into this bug and found the issue is that memcpy that is fortify doesn't work with flexible arrays like QCTL_COPY works with. The infrastructure to support this has landed in the newest kernels; but to support that with every kernel Lustre supports requires way too much back porting. So the solution is not blanket include linux-fortify-string.h since it will break things. Just include it where it is needed.