Re: [dpdk-dev v5 1/2] eal: introduce rte_timingsafe_memcmp() based on OpenBSD API

[Bruce Richardson <bruce.richardson@intel.com>]

On Wed, Oct 01, 2025 at 08:57:02PM +0200, Morten Brørup wrote:
> > From: Kai Ji [mailto:kai.ji@intel.com]
> > Sent: Wednesday, 1 October 2025 17.33
> > 
> > Bugzilla ID: 1773
> > https://bugs.dpdk.org/show_bug.cgi?id=1773
> > 
> > Signed-off-by: Kai Ji <kai.ji@intel.com>
> > ---
> >  lib/eal/include/rte_memory.h | 38 ++++++++++++++++++++++++++++++++++++
> >  1 file changed, 38 insertions(+)
> > 
> > diff --git a/lib/eal/include/rte_memory.h
> > b/lib/eal/include/rte_memory.h
> > index dcc0e69cfe..6939c1caad 100644
> > --- a/lib/eal/include/rte_memory.h
> > +++ b/lib/eal/include/rte_memory.h
> > @@ -746,6 +746,44 @@ __rte_experimental
> >  void
> >  rte_memzero_explicit(void *dst, size_t sz);
> > 
> > +/**
> > + * @warning
> > + * @b EXPERIMENTAL: this API may change without prior notice.
> > + *
> > + * Constant-time memory comparison.
> > + *
> > + * This function compares two memory regions in constant time, making
> > it
> > + * resistant to timing side-channel attacks. The execution time
> > depends only
> > + * on the length parameter, not on the actual data values being
> > compared.
> > + *
> > + * This is particularly important for cryptographic operations where
> > timing
> > + * differences could leak information about secret keys, passwords, or
> > other
> > + * sensitive data.
> > + *
> > + * @param a
> > + *   Pointer to the first memory region to compare
> > + * @param b
> > + *   Pointer to the second memory region to compare
> > + * @param n
> > + *   Number of bytes to compare
> > + * @return
> > + *   0 if the memory regions are identical, non-zero if they differ
> > + */
> > +__rte_experimental
> > +static inline int
> > +rte_timingsafe_memcmp(const void *a, const void *b, size_t n)
> > +{
> > +	const volatile uint8_t *pa = (const volatile uint8_t *)a;
> > +	const volatile uint8_t *pb = (const volatile uint8_t *)b;
> > +	uint8_t result = 0;
> > +	size_t i;
> > +
> > +	for (i = 0; i < n; i++)
> > +		result |= pa[i] ^ pb[i];
> > +
> > +	return result;
> > +}
> > +
> >  #ifdef __cplusplus
> >  }
> >  #endif
> > --
> > 2.34.1
> 
> NAK.
> This returns (binary) non-equality only. It does not return (tri-state) <0, 0, or >0, so it's not like memcmp or FreeBSD timingsafe_memcmp.
> 
> Also, please put the function ("memeq") first in the name, and then the extra property ("timingsafe") last, like rte_memzero_explicit.
> Like this:
> __rte_experimental
> static inline bool
> rte_memeq_timingsafe(const void *a, const void *b, size_t n)
> {
> 	const volatile uint8_t *pa = (const volatile uint8_t *)a;
> 	const volatile uint8_t *pb = (const volatile uint8_t *)b;
> 	uint8_t result = 0;
> 	size_t i;
> 
> 	for (i = 0; i < n; i++)
> 		result |= pa[i] ^ pb[i];
> 
> 	return result == UINT8_C(0);
> }
> 
> Stephen, agree?
> 

Not sure I agree with you on the naming. I'd rather see us adopt the BSD
function (present on multiple BSDs) rather than rolling our own completely
new function with new behaviour.

/Bruce