RE: [EXTERNAL] [dpdk-dev v1] cryptodev: introduce constant-time memory comparison

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

> From: Bruce Richardson [mailto:bruce.richardson@intel.com]
> Sent: Friday, 26 September 2025 09.56
> 
> On Thu, Sep 25, 2025 at 10:47:42PM +0200, Thomas Monjalon wrote:
> > 25/09/2025 12:33, Akhil Goyal:
> > > > +/**
> > > > + * Constant-time memory comparison for cryptographic use.
> > > > + * Returns 0 if the memory regions are equal, nonzero otherwise.
> > > > + * Runs in constant time with respect to the length to prevent
> timing attacks.
> > > > + *
> > > > + * @param a
> > > > + *   Pointer to the first memory region.
> > > > + * @param b
> > > > + *   Pointer to the second memory region.
> > > > + * @param n
> > > > + *   Number of bytes to compare.
> > > > + * @return
> > > > + *   0 if memory regions are equal, nonzero otherwise.
> > > > + */
> > > > +#define rte_consttime_memcmp(a, b, n) __extension__ ({ \
> > > > +	const volatile uint8_t *__pa = (const volatile uint8_t
> *)(a); \
> > > > +	const volatile uint8_t *__pb = (const volatile uint8_t
> *)(b); \
> > > > +	uint8_t __result = 0; \
> > > > +	for (size_t __i = 0; __i < (n); __i++) \
> > > > +		__result |= __pa[__i] ^ __pb[__i]; \
> > > > +	__result; \
> > > > +})
> > >
> > > I believe this is not the right place to add this define.
> > > It should be somewhere in common eal if it is already not there.
> >
> > Yes indeed.
> > cryptodev is the API for managing crypto devices.
> > A new memcmp function would be better hosted in libc,
> > and in EAL for compatibility with all supported libc.
> >
> > I mean please add it in EAL, and propose it to glibc as well.
> >
> 
> Just for reference, there is a good discussion of such functions and
> reference code under MIT license at [1]. After reading that, I note
> that
> the proposed macro above it not strictly a memcmp function because it
> just
> returns a zero/non-zero value, rather than a value indicating which
> array
> value is greater. Therefore some feedback on this code:
> * Use an inline function returning bool rather than a macro
> * A more accurate name might be rte_consttime_memneq, since the code
>   returns 0 (false) if equal.
> 
> Regars,
> /Bruce
> 
> [1] https://github.com/chmike/cst_time_memcmp

When deciding on the function name and location:
Can we foresee other consttime or crypto-purpose functions that should reside in the EAL?

There's already rte_memzero_explicit() in rte_memory.h:
https://elixir.bootlin.com/dpdk/v25.07/source/lib/eal/include/rte_memory.h#L747

Please use the same naming convention, i.e. put the function purpose first, and append the special behavior as postfix:
rte_memneq_consttime(), e.g.:
bool rte_memneq_consttime(const void *s1, const void *s2, size_t n);

IMHO, eal/include/rte_memory.h is a good location for this too.

Should it be an inline or normal function?
If it's primarily for fast path, inline is preferable.

You could also add the inverse function, i.e. add both memneq and memeq.