DPDK patches and discussions
 help / color / mirror / Atom feed
From: Bruce Richardson <bruce.richardson@intel.com>
To: Thomas Monjalon <thomas@monjalon.net>
Cc: Kai Ji <kai.ji@intel.com>, "dev@dpdk.org" <dev@dpdk.org>,
	"Stephen Hemminger" <stephen@networkplumber.org>,
	David Marchand <david.marchand@redhat.com>, <stable@dpdk.org>,
	Pablo de Lara <pablo.de.lara.guarch@intel.com>,
	Fan Zhang <fanzhang.oss@gmail.com>,
	"Akhil Goyal" <gakhil@marvell.com>
Subject: Re: [EXTERNAL] [dpdk-dev v1] cryptodev: introduce constant-time memory comparison
Date: Fri, 26 Sep 2025 08:58:13 +0100	[thread overview]
Message-ID: <aNZHlcgZTCcFkrhn@bricha3-mobl1.ger.corp.intel.com> (raw)
In-Reply-To: <aNZHCww6m8NJ_BHN@bricha3-mobl1.ger.corp.intel.com>

On Fri, Sep 26, 2025 at 08:55:55AM +0100, Bruce Richardson wrote:
> 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.
> 

Or better, change the return value from result to !result to make it a
"memeq" function, which also has the benefit of clamping return values to
just 0 and 1.

> Regars,
> /Bruce
> 
> [1] https://github.com/chmike/cst_time_memcmp

  reply	other threads:[~2025-09-26  7:58 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-09-25 10:22 Kai Ji
2025-09-25 10:33 ` [EXTERNAL] " Akhil Goyal
2025-09-25 20:47   ` Thomas Monjalon
2025-09-26  7:55     ` Bruce Richardson
2025-09-26  7:58       ` Bruce Richardson [this message]
2025-09-26 12:34       ` Morten Brørup
2025-09-26  8:13     ` Konstantin Ananyev
2025-09-26  8:16       ` Konstantin Ananyev
2025-09-26 15:49 ` [dpdk-dev v2 1/2] eal: Add rte_consttime_memsq() to prevent timing attacks memcmp Kai Ji
2025-09-26 15:49   ` [dpdk-dev v2 2/2] crypto/ipsec-mb: use constant-time memory comparison Kai Ji
2025-09-26 16:02   ` [dpdk-dev v3 1/2] eal: Add rte_consttime_memneq() to prevent timing attacks memcmp Kai Ji
2025-09-26 16:02     ` [dpdk-dev v3 2/2] crypto/ipsec-mb: use constant-time memory comparison Kai Ji
2025-09-26 18:12     ` [dpdk-dev v3 1/2] eal: Add rte_consttime_memneq() to prevent timing attacks memcmp Stephen Hemminger
2025-09-26 19:17     ` Morten Brørup
2025-09-26 20:15       ` Stephen Hemminger
2025-09-26 18:07   ` [dpdk-dev v2 1/2] eal: Add rte_consttime_memsq() " Stephen Hemminger

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=aNZHlcgZTCcFkrhn@bricha3-mobl1.ger.corp.intel.com \
    --to=bruce.richardson@intel.com \
    --cc=david.marchand@redhat.com \
    --cc=dev@dpdk.org \
    --cc=fanzhang.oss@gmail.com \
    --cc=gakhil@marvell.com \
    --cc=kai.ji@intel.com \
    --cc=pablo.de.lara.guarch@intel.com \
    --cc=stable@dpdk.org \
    --cc=stephen@networkplumber.org \
    --cc=thomas@monjalon.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).