linux-kernel - Re: [PATCH 5.10 129/593] crypto: qce: skcipher: Fix incorrect sg count for dma transfers

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]

Date:   Thu, 15 Jul 2021 12:46:33 +0200
From:   Greg Kroah-Hartman <gregkh@...uxfoundation.org>
To:     Pavel Machek <pavel@...x.de>
Cc:     linux-kernel@...r.kernel.org, stable@...r.kernel.org,
        Thara Gopinath <thara.gopinath@...aro.org>,
        Herbert Xu <herbert@...dor.apana.org.au>,
        Sasha Levin <sashal@...nel.org>
Subject: Re: [PATCH 5.10 129/593] crypto: qce: skcipher: Fix incorrect sg
 count for dma transfers

On Wed, Jul 14, 2021 at 09:40:28PM +0200, Pavel Machek wrote:
> Hi!
> 
> > [ Upstream commit 1339a7c3ba05137a2d2fe75f602311bbfc6fab33 ]
> > 
> > Use the sg count returned by dma_map_sg to call into
> > dmaengine_prep_slave_sg rather than using the original sg count. dma_map_sg
> > can merge consecutive sglist entries, thus making the original sg count
> > wrong. This is a fix for memory coruption issues observed while testing
> > encryption/decryption of large messages using libkcapi framework.
> > 
> > Patch has been tested further by running full suite of tcrypt.ko tests
> > including fuzz tests.
> 
> This still needs more work AFAICT.
> 
> > index a2d3da0ad95f..5a6559131eac 100644
> > --- a/drivers/crypto/qce/skcipher.c
> > +++ b/drivers/crypto/qce/skcipher.c
> > @@ -122,21 +122,22 @@ qce_skcipher_async_req_handle(struct crypto_async_request *async_req)
> >  	sg_mark_end(sg);
> >  	rctx->dst_sg = rctx->dst_tbl.sgl;
> 
> ret is == 0 at this point.
> 
> > -	ret = dma_map_sg(qce->dev, rctx->dst_sg, rctx->dst_nents, dir_dst);
> > -	if (ret < 0)
> > +	dst_nents = dma_map_sg(qce->dev, rctx->dst_sg, rctx->dst_nents, dir_dst);
> > +	if (dst_nents < 0)
> >  		goto error_free;
> 
> And we go to the error path, and return ret... instead of returning failure.
> 
> >  	if (diff_dst) {
> > -		ret = dma_map_sg(qce->dev, req->src, rctx->src_nents, dir_src);
> > -		if (ret < 0)
> > +		src_nents = dma_map_sg(qce->dev, req->src, rctx->src_nents, dir_src);
> > +		if (src_nents < 0)
> >  			goto error_unmap_dst;
> >  		rctx->src_sg = req->src;
> 
> Same problem happens here.
> 
> The problem is already fixed in the mainline; I believe we want that
> in 5.10-stable at least.
> 
> commit a8bc4f5e7a72e4067f5afd7e98b61624231713ca
> Author: Wei Yongjun <weiyongjun1@...wei.com>
> Date:   Wed Jun 2 11:36:45 2021 +0000
> 
>     crypto: qce - fix error return code in qce_skcipher_async_req_handle()
> 
>     Fix to return a negative error code from the error handling
>         case instead of 0, as done elsewhere in this function.
> 
>     Fixes: 1339a7c3ba05 ("crypto: qce: skcipher: Fix incorrect sg
>     count for dma transfers")
>         Reported-by: Hulk Robot <hulkci@...wei.com>
> 	    Signed-off-by: Wei Yongjun <weiyongjun1@...wei.com>
> 	    
> 

This is also already in this 5.10.50 release.

thanks,

greg k-h