Page MenuHomeFreeBSD

lib/libc/aarch64/string: add timingsafe_memcmp() assembly implementation
AcceptedPublic

Authored by fuz on Sep 23 2024, 9:57 AM.
Tags
None
Referenced Files
F102743696: D46758.diff
Sat, Nov 16, 2:59 PM
Unknown Object (File)
Thu, Nov 14, 9:03 PM
Unknown Object (File)
Mon, Nov 4, 8:04 AM
Unknown Object (File)
Oct 9 2024, 10:59 AM
Unknown Object (File)
Sep 29 2024, 8:43 PM
Unknown Object (File)
Sep 28 2024, 4:22 PM
Unknown Object (File)
Sep 28 2024, 3:33 PM
Unknown Object (File)
Sep 25 2024, 7:55 PM

Details

Reviewers
cperciva
andrew
getz
Group Reviewers
security
Summary

A port of the amd64 implementation (see D41696) with some slight changes due to
differences in instructions provided by aarch64.

No ASIMD for the same reason as the amd64 code: it's just not particularly
suitable for this application.

Event: EuroBSDcon 2024

Please review to ensure that this function fulfills the required constant time
properties. @andrew and @cpercival have agreed to do a joint review of the code
during EuroBSDcon 2024.

We have considered adding a wrapper that would set the DIT (data-independent
timing) bit before the code and reset it to its prior state after, but after
discussion with @imp and others have decided to leave this setting to a future
portable function (i.e. the caller is responsible for enabling DIT mode if
desired).

For benchmarks see D46757.

Test Plan

passes our test suite; test suite does not test constant time
properties.

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Passed
Unit
No Test Coverage
Build Status
Buildable 59565
Build 56452: arc lint + arc unit

Event Timeline

fuz requested review of this revision.Sep 23 2024, 9:57 AM
fuz created this revision.
This revision is now accepted and ready to land.Mon, Oct 21, 6:04 PM

A review with hat security, i.e. by @andrew and @cperciva, is still needed to proceed here.

lib/libc/aarch64/string/timingsafe_memcmp.S
34

If len == 2 then we're subtracting a big-endian 16-bit value from a big-endian 16-bit value here, right? But memcmp is supposed to return the difference between the first differing bytes; so memcmp("aa", "ab", 2) should return 1, not 0x100 which I think is what this does?

lib/libc/aarch64/string/timingsafe_memcmp.S
34

Unlike memcmp, the timingsafe_memcmp function is not documented to return the difference between the two values, but rather just a positive, zero, or negative value. As it is very difficult to efficiently return the exact difference in a timingsafe manner, I have decided against trying to go beyond the specified behavior.

Not only that, standard memcmp is only specified to return a -ve, 0, or +ve number and it's not portable to rely on an implementation returning the difference. I don't see any reason we'd need to provide that behaviour on a new function.