Re: [BUG] crypto: ccree: driver does not handle case where cryptlen = authsize =0

From: Corentin Labbe
Date: Fri Mar 04 2022 - 08:30:17 EST

Next message: Neil Armstrong: "Re: [Letux-kernel] [PATCH v16 1/4] drm/bridge: dw-hdmi: introduce dw_hdmi_enable_poll()"
Previous message: Rob Herring: "Re: [PATCH] dt-bindings: clock: renesas: Make example 'clocks' parsable"
Next in thread: Herbert Xu: "Re: [BUG] crypto: ccree: driver does not handle case where cryptlen = authsize =0"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]

Le Mon, Feb 28, 2022 at 11:11:43AM +0200, Gilad Ben-Yossef a écrit :
> Hi,
>
> On Tue, Feb 22, 2022 at 9:39 AM Gilad Ben-Yossef <gilad@xxxxxxxxxxxxx> wrote:
> >
> > On Mon, Feb 21, 2022 at 4:05 PM Corentin Labbe
> > <clabbe.montjoie@xxxxxxxxx> wrote:
> > >
> > > Le Mon, Feb 21, 2022 at 12:08:12PM +0200, Gilad Ben-Yossef a écrit :
> > > > Hi,
> > > >
> > > > On Sun, Feb 20, 2022 at 9:26 PM Corentin Labbe
> > > > <clabbe.montjoie@xxxxxxxxx> wrote:
> > > > >
> > > > ...
> > > > >
> > > > > Hello
> > > > >
> > > > > While testing your patch for this problem, I saw another warning (unrelated with your patch):
> > > >
> > > > Dear Corentin, you are a treasure trove of bug reports. I love it.
> > > > Thank you! :-)
> > > >
> > > > > [ 34.061953] ------------[ cut here ]------------
> > ...
> > > >
> > > > So, this is an interesting one.
> > > > What I *think* is happening is that the drbg implementation is
> > > > actually doing something naughty: it is passing the same exact memory
> > > > buffer, both as source and destination to an encryption operation to
> > > > the crypto skcipher API, BUT via two different scatter gather lists.
> > > >
> > > > I'm not sure but I believe this is not a legitimate use of the API,
> > > > but before we even go into this, let's see if this little fix helps at
> > > > all and this is indeed the root cause.
> > > >
> > > > Can you test this small change for me, please?
> > > >
> > > > diff --git a/crypto/drbg.c b/crypto/drbg.c
> > > > index 177983b6ae38..13824fd27627 100644
> > > > --- a/crypto/drbg.c
> > > > +++ b/crypto/drbg.c
> > > > @@ -1851,7 +1851,7 @@ static int drbg_kcapi_sym_ctr(struct drbg_state *drbg,
> > > > /* Use scratchpad for in-place operation */
> > > > inlen = scratchpad_use;
> > > > memset(drbg->outscratchpad, 0, scratchpad_use);
> > > > - sg_set_buf(sg_in, drbg->outscratchpad, scratchpad_use);
> > > > + sg_in = sg_out;
> > > > }
> > > >
> > > > while (outlen) {
> > > >
> > >
> > > No more stacktrace !
> >
> > Thank you. I will send a patch later today.
>
> > --
> > Gilad Ben-Yossef
> > Chief Coffee Drinker
> >
> > values of β will give rise to dom!
>
> OK, it seems my direction of fixing the caller site has not been taken
> kindly by the power that be.
> Let's try something else.
>
> Can you please drop the previous patch and test this one instead?
>
> diff --git a/drivers/crypto/ccree/cc_buffer_mgr.c
> b/drivers/crypto/ccree/cc_buffer_mgr.c
> index 11e0278c8631..398843040566 100644
> --- a/drivers/crypto/ccree/cc_buffer_mgr.c
> +++ b/drivers/crypto/ccree/cc_buffer_mgr.c
> @@ -377,6 +377,7 @@ int cc_map_cipher_request(struct cc_drvdata
> *drvdata, void *ctx,
> u32 dummy = 0;
> int rc = 0;
> u32 mapped_nents = 0;
> + int src_direction = (src != dst ? DMA_TO_DEVICE : DMA_BIDIRECTIONAL);
>
> req_ctx->dma_buf_type = CC_DMA_BUF_DLLI;
> mlli_params->curr_pool = NULL;
> @@ -399,7 +400,7 @@ int cc_map_cipher_request(struct cc_drvdata
> *drvdata, void *ctx,
> }
>
> /* Map the src SGL */
> - rc = cc_map_sg(dev, src, nbytes, DMA_BIDIRECTIONAL, &req_ctx->in_nents,
> + rc = cc_map_sg(dev, src, nbytes, src_direction, &req_ctx->in_nents,
> LLI_MAX_NUM_OF_DATA_ENTRIES, &dummy, &mapped_nents);
> if (rc)
> goto cipher_exit;
> @@ -416,7 +417,7 @@ int cc_map_cipher_request(struct cc_drvdata
> *drvdata, void *ctx,
> }
> } else {
> /* Map the dst sg */
> - rc = cc_map_sg(dev, dst, nbytes, DMA_BIDIRECTIONAL,
> + rc = cc_map_sg(dev, dst, nbytes, DMA_FROM_DEVICE,
> &req_ctx->out_nents, LLI_MAX_NUM_OF_DATA_ENTRIES,
> &dummy, &mapped_nents);
> if (rc)
>
>
> Thanks!
> Gilad
>

Hello

I got:
[ 17.563793] ------------[ cut here ]------------
[ 17.568492] DMA-API: ccree e6601000.crypto: device driver frees DMA memory with different direction [device address=0x0000000078fe5800] [size=8 bytes] [mapped with DMA_TO_DEVICE] [unmapped with DMA_BIDIRECTIONAL]
[ 17.587371] WARNING: CPU: 0 PID: 0 at /home/clabbe/linux-next/kernel/dma/debug.c:1018 check_unmap+0x138/0x868
[ 17.597304] Modules linked in:
[ 17.600364] CPU: 0 PID: 0 Comm: swapper/0 Not tainted 5.17.0-rc6-next-20220303-00130-g30042e47ee47-dirty #52
[ 17.610191] Hardware name: Renesas Salvator-X board based on r8a77950 (DT)
[ 17.617063] pstate: 600000c5 (nZCv daIF -PAN -UAO -TCO -DIT -SSBS BTYPE=--)
[ 17.624026] pc : check_unmap+0x138/0x868
[ 17.627948] lr : check_unmap+0x138/0x868
[ 17.631870] sp : ffff80000b4037a0
[ 17.635183] x29: ffff80000b4037a0 x28: ffff0004c8789840 x27: 0000000000000000
[ 17.642329] x26: ffff80000a9b6000 x25: ffff80000c4ea030 x24: 0000000000000000
[ 17.649473] x23: ffff80000b420000 x22: ffff80000b4c7000 x21: ffff80000c4c2000
[ 17.656617] x20: ffff80000b403880 x19: ffff0004c0ad2880 x18: ffffffffffffffff
[ 17.663760] x17: 63697665645b206e x16: 6f69746365726964 x15: 00000000000001b5
[ 17.670904] x14: ffff80000b403490 x13: 00000000ffffffea x12: ffff80000b4be010
[ 17.678048] x11: 0000000000000001 x10: 0000000000000001 x9 : ffff80000b4a6028
[ 17.685191] x8 : c0000000ffffefff x7 : 0000000000017fe8 x6 : ffff80000b4a5fd0
[ 17.692334] x5 : 0000000000057fa8 x4 : 0000000000000000 x3 : 00000000ffffefff
[ 17.699477] x2 : ffff80000b44dda8 x1 : beaec6c6f6c7ed00 x0 : 0000000000000000
[ 17.706621] Call trace:
[ 17.709065] check_unmap+0x138/0x868
[ 17.712641] debug_dma_unmap_sg+0xfc/0x1c8
[ 17.716740] dma_unmap_sg_attrs+0x4c/0xa8
[ 17.720755] cc_unmap_cipher_request+0x50/0xb0
[ 17.725206] cc_cipher_complete+0x44/0x80
[ 17.729217] comp_handler+0x178/0x3a8
[ 17.732881] tasklet_action_common.isra.15+0x148/0x160
[ 17.738025] tasklet_action+0x28/0x38
[ 17.741686] __do_softirq+0x13c/0x5ec
[ 17.745350] irq_exit_rcu+0x18c/0x1b0
[ 17.749012] el1_interrupt+0x44/0x78
[ 17.752594] el1h_64_irq_handler+0x18/0x28
[ 17.756692] el1h_64_irq+0x64/0x68
[ 17.760093] cpuidle_enter_state+0x104/0x528
[ 17.764367] cpuidle_enter+0x3c/0x58
[ 17.767944] call_cpuidle+0x20/0x50
[ 17.771437] do_idle+0x23c/0x298
[ 17.774666] cpu_startup_entry+0x24/0x80
[ 17.778591] rest_init+0x184/0x288
[ 17.781995] arch_call_rest_init+0x10/0x1c
[ 17.786097] start_kernel+0x6dc/0x718
[ 17.789759] __primary_switched+0xc0/0xc8
[ 17.793769] irq event stamp: 2082825
[ 17.797342] hardirqs last enabled at (2082824): [<ffff800009caa0e4>] _raw_spin_unlock_irqrestore+0x8c/0x90
[ 17.807087] hardirqs last disabled at (2082825): [<ffff800009caa6a0>] _raw_spin_lock_irqsave+0xb8/0xc8
[ 17.816394] softirqs last enabled at (2082814): [<ffff800008010550>] __do_softirq+0x4a8/0x5ec
[ 17.825006] softirqs last disabled at (2082819): [<ffff8000080abb6c>] irq_exit_rcu+0x18c/0x1b0
[ 17.833616] ---[ end trace 0000000000000000 ]---
[ 17.838232] DMA-API: Mapped at:
[ 17.841371] debug_dma_map_sg+0x16c/0x398
[ 17.845382] __dma_map_sg_attrs+0x9c/0x108
[ 17.849480] dma_map_sg_attrs+0x10/0x28

I got also the overlapping message on one of my driver now, but it is hard to debug, I dont see how it can happen.

Next message: Neil Armstrong: "Re: [Letux-kernel] [PATCH v16 1/4] drm/bridge: dw-hdmi: introduce dw_hdmi_enable_poll()"
Previous message: Rob Herring: "Re: [PATCH] dt-bindings: clock: renesas: Make example 'clocks' parsable"
Next in thread: Herbert Xu: "Re: [BUG] crypto: ccree: driver does not handle case where cryptlen = authsize =0"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]