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
F106413228: D46758.diff
Mon, Dec 30, 8:07 AM
Unknown Object (File)
Sun, Dec 29, 8:37 AM
Unknown Object (File)
Sat, Dec 28, 8:32 AM
Unknown Object (File)
Fri, Dec 27, 12:33 PM
Unknown Object (File)
Tue, Dec 24, 10:25 PM
Unknown Object (File)
Nov 29 2024, 12:29 PM
Unknown Object (File)
Nov 16 2024, 5:49 PM
Unknown Object (File)
Nov 16 2024, 2:59 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.Oct 21 2024, 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.

LGTM. Sorry it took so long for me to find time.

Thank you for taking your time to review this one.