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

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

On Thu, Oct 02, 2025 at 10:37:09AM +0200, Morten Brørup wrote:
> > 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().
> 
Ok, point taken.