linux-kernel - Re: [PATCH v0] Crypto: Talitos: re-initialize async

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [day] [month] [year] [list]

Message-ID: <4B27D42C.8070309@intel.com>
Date:	Tue, 15 Dec 2009 11:23:40 -0700
From:	Dan Williams <dan.j.williams@...el.com>
To:	Suresh Vishnu-B05022 <B05022@...escale.com>
CC:	"linuxppc-dev@...abs.org" <linuxppc-dev@...abs.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"linux-raid@...r.kernel.org" <linux-raid@...r.kernel.org>,
	"linux-crypto@...r.kernel.org" <linux-crypto@...r.kernel.org>,
	"herbert@...dor.apana.org.au" <herbert@...dor.apana.org.au>,
	Tabi Timur-B04825 <B04825@...escale.com>,
	Li Yang-R58472 <r58472@...escale.com>,
	Ira Snyder <iws@...o.caltech.edu>
Subject: Re: [PATCH v0] Crypto: Talitos: re-initialize async_tx descriptors

Suresh Vishnu-B05022 wrote:
>  > On Mon, Dec 14, 2009 at 6:33 AM, Vishnu Suresh <Vishnu@...escale.com> 
> wrote:
>  > > The async_tx descriptors contains dangling pointers.
>  > > Hence, re-initialize them to NULL before use.
>  > >
>  > > Signed-off-by: Vishnu Suresh <Vishnu@...escale.com>
>  > > ---
>  > > o. Rebased to linux-next as of 20091214
>  > >
>  > >  drivers/crypto/talitos.c |    3 +++
>  > >  1 files changed, 3 insertions(+), 0 deletions(-)
>  > >
>  > > diff --git a/drivers/crypto/talitos.c b/drivers/crypto/talitos.c
>  > > index 87f06be..9e261c6 100644
>  > > --- a/drivers/crypto/talitos.c
>  > > +++ b/drivers/crypto/talitos.c
>  > > @@ -952,6 +952,9 @@ static struct dma_async_tx_descriptor * 
> talitos_prep_dma_xor(
>  > >                return NULL;
>  > >        }
>  > >        dma_async_tx_descriptor_init(&new->async_tx, &xor_chan->common);
>  > > +       new->async_tx.parent = NULL;
>  > > +       new->async_tx.next = NULL;
>  > > +
>  > >
>  > >        desc = &new->hwdesc;
>  > >        /* Set destination: Last pointer pair */
> 
>  > These two values are owned by the async_tx api, drivers are not
>  > supposed to touch them.
> I have sent this patch and the similar one for fsldma seperately,
> so that if the changes are needed and can be done in 
> dma_async_tx_descriptor_init(),
> these patches can be ignored. 
>  > Both iop_adma and the new ppx4xx driver
>  > (which use the async_tx channel switching capability) get away without
>  > touching these fields which makes me suspect there is a
>  > misunderstanding/bug somewhere else in the talitos implementation.
> 
> This bug does not occur on all the platforms. The occurrance is random.
> This occurs when a channel switch between two different devices are 
> present.
> This same initialization is required in case of fsldma as well. 
> In case of fsldma/talitosXOR, there are two DMA channels (on same 
> device) and single XOR channel (on another device).
> 
> When used without fsldma, this driver works fine.
> Does iop_adma and ppx4xx work in similar enviroment?

Yes, iop_adma when running on iop3xx hardware has one xor channel and 
two memcpy channels.

The bug may very well be in the fsldma driver, but the workarounds are 
just band aids.  I would be more comfortable with dropping the three 
workaround patches and simply adding "depends on !FSL_DMA" to the 
CRYPTO_DEV_TALITOS option until the true fix can be developed.

--
Dan

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/