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.j...@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 <pkochanow...@sii.pl>
> > Sent: Monday, April 7, 2025 6:17 PM
> > To: u-boot@lists.denx.de
> > Cc: Priyanka Jain <priyanka.j...@nxp.com>; Gaurav Jain
> > <gaurav.j...@nxp.com>; Gabriel Nesteruk <gneste...@sii.pl>; Paweł
> > Kochanowski <pkochanow...@sii.pl>
> > Subject: [EXT] [PATCH] crypto: fsl - Fix RNG generation for lengths
> > greater than 16 bytes
> >
> > [You don't often get email from pkochanow...@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 <gneste...@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 <gneste...@sii.pl>
> > Signed-off-by: Pawel Kochanowski <pkochanow...@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
