Re: ixp4xx_crypto: Fix possible NULL ptr dereference.

[Christian Hohnstaedt <chohnstaedt@xxxxxxxxxxxxxx>]

On Sun, Jan 10, 2010 at 06:37:25PM +0100, Krzysztof Halasa wrote:
> Signed-off-by: Krzysztof Hałasa <khc@xxxxxxxxx>
> 
> diff --git a/drivers/crypto/ixp4xx_crypto.c b/drivers/crypto/ixp4xx_crypto.c
> index f8f6515..2ae7148 100644
> --- a/drivers/crypto/ixp4xx_crypto.c
> +++ b/drivers/crypto/ixp4xx_crypto.c
> @@ -786,10 +786,8 @@ static struct buffer_desc *chainup_buffers(struct device *dev,
>  		nbytes -= len;
>  		ptr = page_address(sg_page(sg)) + sg->offset;
>  		next_buf = dma_pool_alloc(buffer_pool, flags, &next_buf_phys);
> -		if (!next_buf) {
> -			buf = NULL;
> -			break;
> -		}
> +		if (!next_buf)
> +			return NULL;

This leaves buf->next uninitialized, but
free_buf_chain() iterates over buf->next.

We need:

	if (!next_buf) {
		buf->next = NULL;
		return NULL;
	}

Or get rid of next_buf and next_buf_phys:

diff --git a/drivers/crypto/ixp4xx_crypto.c b/drivers/crypto/ixp4xx_crypto.c
index b8cc714..c961b0f 100644
--- a/drivers/crypto/ixp4xx_crypto.c
+++ b/drivers/crypto/ixp4xx_crypto.c
@@ -794,21 +794,15 @@ static struct buffer_desc *chainup_buffers(struct device *dev,
 {
 	for (;nbytes > 0; sg = scatterwalk_sg_next(sg)) {
 		unsigned len = min(nbytes, sg->length);
-		struct buffer_desc *next_buf;
-		u32 next_buf_phys;
 		void *ptr;
 
 		nbytes -= len;
 		ptr = page_address(sg_page(sg)) + sg->offset;
-		next_buf = dma_pool_alloc(buffer_pool, flags, &next_buf_phys);
-		if (!next_buf) {
-			buf = NULL;
-			break;
-		}
+		buf->next = dma_pool_alloc(buffer_pool, flags, &buf->phys_next);
+		if (!buf->next)
+			return NULL;
 		sg_dma_address(sg) = dma_map_single(dev, ptr, len, dir);
-		buf->next = next_buf;
-		buf->phys_next = next_buf_phys;
-		buf = next_buf;
+		buf = buf->next;
 
 		buf->phys_addr = sg_dma_address(sg);
 		buf->buf_len = len;


Christian Hohnstaedt

-- 
Christian Hohnstaedt / Project Manager Hardware and Manufacturing

Innominate Security Technologies AG / protecting industrial networks
tel: +49.30.921028.208 / fax: +49.30.921028.020
Rudower Chaussee 13, D-12489 Berlin / http://www.innominate.com

Register Court: AG Charlottenburg, HR B 81603
Management Board: Dirk Seewald
Chairman of the Supervisory Board: Volker Bibelhausen
--
To unsubscribe from this list: send the line "unsubscribe linux-crypto" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html