[EXT] [PATCH] crypto: fsl - Fix RNG generation for lengths greater than 16 bytes

[Paweł Kochanowski]

Hi Gaurav,

What we see is that the jr_enqueue() called by run_descriptor_jr() swaps the endianness of the descriptor in place (by modifying the data pointed by desc_add):

	/* The descriptor must be submitted to SEC block as per endianness
	 * of the SEC Block.
	 * So, if the endianness of Core and SEC block is different, each word
	 * of the descriptor will be byte-swapped.
	 */
	for (i = 0; i < length; i++) {
		desc_word = desc_addr[i];
		sec_out32((uint32_t *)&desc_addr[i], desc_word);
	}

So the sequence looks like this:
1. caam_rng_probe sets correct descriptor in ` caam_rng_priv *priv`
2. caam_rng_read is called with 32B
3. caam_rng_read_one->run_descriptor_jr() is called to generate 16B with `priv->desc` containing valid descriptor
4. The descriptor is swapped in jr_enqueue() before passing it to job ring
5. The loop in caam_rng_read is called second time, this time the `priv->desc` contain swapped data.

Interesting thing is that the job still succeeds and that some data are present in the buffers, but maybe swapped descriptor can also be a valid one?
The effect that we see is that when leaving the command executing this sequence we get an exception in "unrelated" place..

This is what we see when printing descriptor in the caam_rng_read_one on LS1046A:

First iteration:
0: B0800005
1: 82500002
2: 60340010
3: 0
4: FBC94DC0

Second iteration:
0: 050080B0
1: 02005082
2: 10003460
3: 0
4: C04DC9FB

BR,
Pawel.

> From: Gaurav Jain <gaurav.jain at nxp.com>
> Sent: Tuesday, April 8, 2025 12:09 PM
> 
> Can you explain more details about this issue. Where it is being modified in
> run_descriptor_jr() ?
> Regards
> Gaurav
> 
> > -----Original Message-----
> > From: Pawel Kochanowski <pkochanowski at sii.pl>
> > Sent: Monday, April 7, 2025 6:17 PM
> > To: u-boot at lists.denx.de
> > Cc: Priyanka Jain <priyanka.jain at nxp.com>; Gaurav Jain
> > <gaurav.jain at nxp.com>; Gabriel Nesteruk <gnesteruk at sii.pl>; Paweł
> > Kochanowski <pkochanowski at sii.pl>
> > Subject: [EXT] [PATCH] crypto: fsl - Fix RNG generation for lengths
> > greater than 16 bytes
> >
> > [You don't often get email from pkochanowski at sii.pl. Learn why this is
> > important at https://aka.ms/LearnAboutSenderIdentification ]
> >
> > Caution: This is an external email. Please take care when clicking
> > links or opening attachments. When in doubt, report the message using the
> 'Report this email'
> > button
> >
> >
> > From: Gabriel Nesteruk <gnesteruk at sii.pl>
> >
> > Reinitialize the descriptor for each RNG job, as it may be modified by
> > run_descriptor_jr().
> > Failing to do so can result in memory corruption when
> > dm_rng_read() is called a second time on NXP devices with
> > CONFIG_SYS_FSL_SEC_BE enabled.
> >
> > Signed-off-by: Gabriel Nesteruk <gnesteruk at sii.pl>
> > Signed-off-by: Pawel Kochanowski <pkochanowski at sii.pl>
> > ---
> >  drivers/crypto/fsl/rng.c | 14 ++++++--------
> >  1 file changed, 6 insertions(+), 8 deletions(-)
> >
> > diff --git a/drivers/crypto/fsl/rng.c b/drivers/crypto/fsl/rng.c index
> > 06364948052..440b26e3c94 100644
> > --- a/drivers/crypto/fsl/rng.c
> > +++ b/drivers/crypto/fsl/rng.c
> > @@ -27,8 +27,14 @@ struct caam_rng_priv {  static int
> > caam_rng_read_one(struct caam_rng_priv *priv)  {
> >         int size = ALIGN(CAAM_RNG_MAX_FIFO_STORE_SIZE,
> > ARCH_DMA_MINALIGN);
> > +       ulong rng_desc_size = ALIGN(CAAM_RNG_DESC_LEN,
> > + ARCH_DMA_MINALIGN);
> >         int ret;
> >
> > +       inline_cnstr_jobdesc_rng(priv->desc, priv->data,
> > +               CAAM_RNG_MAX_FIFO_STORE_SIZE);
> > +       flush_dcache_range((unsigned long)priv->desc,
> > +         (unsigned long)priv->desc + rng_desc_size);
> > +
> >         ret = run_descriptor_jr(priv->desc);
> >         if (ret < 0)
> >                 return -EIO;
> > @@ -63,14 +69,6 @@ static int caam_rng_read(struct udevice *dev, void
> > *data, size_t len)
> >
> >  static int caam_rng_probe(struct udevice *dev)  {
> > -       struct caam_rng_priv *priv = dev_get_priv(dev);
> > -       ulong size = ALIGN(CAAM_RNG_DESC_LEN, ARCH_DMA_MINALIGN);
> > -
> > -       inline_cnstr_jobdesc_rng(priv->desc, priv->data,
> > -                                CAAM_RNG_MAX_FIFO_STORE_SIZE);
> > -       flush_dcache_range((unsigned long)priv->desc,
> > -                          (unsigned long)priv->desc + size);
> > -
> >         return 0;
> >  }
> >
> > --
> > 2.43.0