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

[Gaurav Jain]

Hi Pawel

> -----Original Message-----
> From: Paweł Kochanowski <pkochanowski at sii.pl>
> Sent: Tuesday, April 8, 2025 4:30 PM
> To: Gaurav Jain <gaurav.jain at nxp.com>; u-boot at lists.denx.de
> Cc: Priyanka Jain <priyanka.jain at nxp.com>; Gabriel Nesteruk
> <gnesteruk at sii.pl>; Pankaj Gupta <pankaj.gupta at nxp.com>; Horia Geanta
> <horia.geanta at nxp.com>
> Subject: RE: [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
> 
> 
> 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

I see you are right. In Linux, caam endianness is handled at the time of creating the descriptor.
But In Uboot, caam endianness is not handled at the time of descriptor creation.

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?
I agree that the descriptor should be reinitialized for each RNG job.
I am also not sure how the swapped descriptor worked second time.

> The effect that we see is that when leaving the command executing this sequence
> we get an exception in "unrelated" place..
After your change you stop seeing this exception? Are you sure it is related to rng descriptor?

> 
> 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
4th word is seqoutptr: sgf pre ext (ews). Have you also modified the descriptor ?

Regards
Gaurav
> 
> 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