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

["Morten Brørup" <mb@smartsharesystems.com>]

> From: Bruce Richardson [mailto:bruce.richardson@intel.com]
> Sent: Thursday, 2 October 2025 10.10
> 
> 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.

I don't see any use for timing safe greater/less than comparison, only equality comparison.
So I assume you are referring to NetBSD's consttime_memequal()?

Regarding the name of the function, I prefer consistent naming across DPDK, rather than aligning the names of a few functions that happen to exist in BSD with their names there.
What happens when BSD introduces functions that already exist in DPDK?
Should we rename rte_memzero_explicit() to rte_explicit_bzero() to align with BSD?

I prefer rte_memeq_timingsafe() or rte_memeq_consttime(), but will not object to rte_consttime_memequal().

> 
> /Bruce