linux-kernel - RE: [PATCH v7 1/8] Talitos: Support for 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>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <BCB48C05FCE8BC4D9E61E841ECBE6DB70FDD90@039-SN2MPN1-011.039d.mgd.msft.net>
Date:	Fri, 31 Aug 2012 10:41:08 +0000
From:	Liu Qiang-B32616 <B32616@...escale.com>
To:	Geanta Neag Horia Ioan-B05471 <B05471@...escale.com>,
	"linux-crypto@...r.kernel.org" <linux-crypto@...r.kernel.org>,
	"dan.j.williams@...il.com" <dan.j.williams@...il.com>,
	"herbert@...dor.hengli.com.au" <herbert@...dor.hengli.com.au>,
	"davem@...emloft.net" <davem@...emloft.net>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"linuxppc-dev@...ts.ozlabs.org" <linuxppc-dev@...ts.ozlabs.org>
CC:	Li Yang-R58472 <r58472@...escale.com>,
	Phillips Kim-R1AAHA <R1AAHA@...escale.com>,
	"vinod.koul@...el.com" <vinod.koul@...el.com>,
	Dan Williams <djbw@...com>, "arnd@...db.de" <arnd@...db.de>,
	"gregkh@...uxfoundation.org" <gregkh@...uxfoundation.org>
Subject: RE: [PATCH v7 1/8] Talitos: Support for async_tx XOR offload

> -----Original Message-----
> From: Geanta Neag Horia Ioan-B05471
> Sent: Friday, August 31, 2012 6:39 PM
> To: Liu Qiang-B32616; linux-crypto@...r.kernel.org;
> dan.j.williams@...il.com; herbert@...dor.hengli.com.au;
> davem@...emloft.net; linux-kernel@...r.kernel.org; linuxppc-
> dev@...ts.ozlabs.org
> Cc: Li Yang-R58472; Phillips Kim-R1AAHA; vinod.koul@...el.com; Dan
> Williams; arnd@...db.de; gregkh@...uxfoundation.org
> Subject: RE: [PATCH v7 1/8] Talitos: Support for async_tx XOR offload
> 
> On Fri, 31 Aug 2012 06:08:05 +0300, Liu Qiang-B32616
> <B32616@...escale.com>
> wrote:
> > > -----Original Message-----
> > > From: Geanta Neag Horia Ioan-B05471
> > > Sent: Thursday, August 30, 2012 10:23 PM
> > > To: Liu Qiang-B32616; linux-crypto@...r.kernel.org;
> > > dan.j.williams@...il.com; herbert@...dor.hengli.com.au;
> > > davem@...emloft.net; linux-kernel@...r.kernel.org; linuxppc-
> > > dev@...ts.ozlabs.org
> > > Cc: Li Yang-R58472; Phillips Kim-R1AAHA; vinod.koul@...el.com;
> > > dan.j.williams@...el.com; arnd@...db.de; gregkh@...uxfoundation.org;
> Liu
> > > Qiang-B32616
> > > Subject: RE: [PATCH v7 1/8] Talitos: Support for async_tx XOR offload
> > >
> > > On Thu, 9 Aug 2012 11:20:48 +0300, qiang.liu@...escale.com wrote:
> > > > From: Qiang Liu <qiang.liu@...escale.com>
> > > >
> > > > Expose Talitos's XOR functionality to be used for RAID parity
> > > > calculation via the Async_tx layer.
> > > >
> > > > Cc: Herbert Xu <herbert@...dor.apana.org.au>
> > > > Cc: David S. Miller <davem@...emloft.net>
> > > > Signed-off-by: Dipen Dudhat <Dipen.Dudhat@...escale.com>
> > > > Signed-off-by: Maneesh Gupta <Maneesh.Gupta@...escale.com>
> > > > Signed-off-by: Kim Phillips <kim.phillips@...escale.com>
> > > > Signed-off-by: Vishnu Suresh <Vishnu@...escale.com>
> > > > Signed-off-by: Qiang Liu <qiang.liu@...escale.com>
> > > > ---
> > > >  drivers/crypto/Kconfig   |    9 +
> > > >  drivers/crypto/talitos.c |  413
> > > ++++++++++++++++++++++++++++++++++++++++++++++
> > > >  drivers/crypto/talitos.h |   53 ++++++
> > > >  3 files changed, 475 insertions(+), 0 deletions(-)
> > >
> 
> > > > +static int talitos_alloc_chan_resources(struct dma_chan *chan)
> > > > +{
> > > > +	struct talitos_xor_chan *xor_chan;
> > > > +	struct talitos_xor_desc *desc;
> > > > +	LIST_HEAD(tmp_list);
> > > > +	int i;
> > > > +
> > > > +	xor_chan = container_of(chan, struct talitos_xor_chan,
> common);
> > > > +
> > > > +	if (!list_empty(&xor_chan->free_desc))
> > > > +		return xor_chan->total_desc;
> > > > +
> > > > +	for (i = 0; i < TALITOS_MAX_DESCRIPTOR_NR; i++) {
> > > > +		desc = talitos_xor_alloc_descriptor(xor_chan,
> > > > +				GFP_KERNEL | GFP_DMA);
> > >
> > > talitos_xor_alloc_descriptor() is called here without holding
> > > the xor_chan->desc_lock and it increments xor_chan->total_desc.
> > > Isn't this an issue ?
> >
> > No, please refer to the code as below,
> > +	list_add_tail(&desc->node, &tmp_list);
> >
> > The list is temporary list, it will be merged to xor_chan->free_desc in
> next step, here is protected
> > by lock,
> > +	spin_lock_bh(&xor_chan->desc_lock);
> > +	list_splice_init(&tmp_list, &xor_chan->free_desc);
> > +	spin_unlock_bh(&xor_chan->desc_lock);
> 
> I was not referring to the list, but to xor_chan->total_desc variable.
> The following access:
> talitos_alloc_chan_resources()->talitos_xor_alloc_descriptor()-
> >total_desc++
> is not protected by the xor_chan->desc_lock.
Ok, I will correct it in next series. Thanks.


--
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/