lib/libc/amd64/string: fix overread condition in memccpy

[fuz]

Summary

An overread condition in memccpy(dst, src, c, len) would occur if
src does not cross a 16 byte boundary and there is no instance of
c between *src and the next 16 byte boundary. This could cause a
read fault if src is just before the end of a page and the next page
is unmapped or unreadable.

The bug is a consequence of basing memccpy() on the strlcpy() code:
whereas strlcpy() assumes that src is a nul-terminated string and
hence a terminator is always present, c may not be present at all in
the source string. It was not caught earlier due to insufficient
unit test design.

Performance is reduced slighty as a result of the extra check.

os: FreeBSD
arch: amd64
cpu: 11th Gen Intel(R) Core(TM) i7-1165G7 @ 2.80GHz
        │ memccpy.unfixed.out │         memccpy.fixed.out          │
        │       sec/op        │   sec/op     vs base               │
Short             66.76µ ± 0%   68.37µ ± 1%  +2.42% (p=0.000 n=20)
Mid               7.938µ ± 0%   7.996µ ± 1%  +0.73% (p=0.003 n=20)
Long              3.577µ ± 0%   3.576µ ± 0%  -0.02% (p=0.005 n=20)
geomean           12.38µ        12.50µ       +1.04%

        │ memccpy.unfixed.out │          memccpy.fixed.out          │
        │         B/s         │     B/s       vs base               │
Short            1.744Gi ± 0%   1.703Gi ± 1%  -2.36% (p=0.000 n=20)
Mid              14.67Gi ± 0%   14.56Gi ± 1%  -0.72% (p=0.003 n=20)
Long             32.55Gi ± 0%   32.55Gi ± 0%  +0.02% (p=0.005 n=20)
geomean          9.407Gi        9.310Gi       -1.03%

Reported by: @getz
See also: D46051
MFC: stable/14

Test Plan

See D46051.

[fuz]

I may have to rework this one. The changed limit check means that the code now fails with phony size limits (i.e. len == SIZE_MAX). I'll have to see how to solve this one without sacrificing more performance.

[fuz]

• lib/libc/amd64/string/memccpy.S: improve performance

The performance is now 6.9% faster than how it was before the bug fix
for the short case and only 0.36% slower for the mid case. The long
case is unchanged. The bug remains fixed.

Please review this change set, I don't plan to update it further.

os: FreeBSD
arch: amd64
cpu: 11th Gen Intel(R) Core(TM) i7-1165G7 @ 2.80GHz
        │ memccpy.unfixed.out │        memccpy.fixed.7.out         │
        │       sec/op        │   sec/op     vs base               │
Short             66.76µ ± 0%   62.45µ ± 1%  -6.44% (p=0.000 n=20)
Mid               7.938µ ± 0%   7.967µ ± 0%  +0.36% (p=0.001 n=20)
Long              3.577µ ± 0%   3.577µ ± 0%       ~ (p=0.429 n=20)
geomean           12.38µ        12.12µ       -2.08%

        │ memccpy.unfixed.out │         memccpy.fixed.7.out         │
        │         B/s         │     B/s       vs base               │
Short            1.744Gi ± 0%   1.864Gi ± 1%  +6.89% (p=0.000 n=20)
Mid              14.67Gi ± 0%   14.61Gi ± 0%  -0.36% (p=0.001 n=20)
Long             32.55Gi ± 0%   32.55Gi ± 0%       ~ (p=0.429 n=20)
geomean          9.407Gi        9.606Gi       +2.12%

[getz]

Looks good, gonna revise the aarch64 port of this to handle it the same way.