linux-kernel - Re: [PATCH v2] scatterlist: prevent invalid free when alloc fails

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 for Android: free password hash cracker in your pocket

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

Date:	Mon, 30 Aug 2010 18:00:21 +0200
From:	Tejun Heo <tj@...nel.org>
To:	Jeffrey Carlyle <jeff.carlyle@...orola.com>
CC:	OLUSANYA SOYANNWO <olusanya.soyannwo@...orola.com>,
	linux-kernel@...r.kernel.org, Hu Tao <taohu@...orola.com>,
	jaxboe@...ionio.com, torvalds@...l.org
Subject: Re: [PATCH v2] scatterlist: prevent invalid free when alloc fails

Hello,

On 08/30/2010 05:01 PM, Jeffrey Carlyle wrote:
> When alloc fails, free_table is being called. Depending on the number of
> bytes requested, we determine if we are going to call _get_free_page()
> or kmalloc(). When alloc fails, our math is wrong (due to sg_size - 1),
> and the last buffer is wrongfully assumed to have been allocated by
> kmalloc. Hence, kfree gets called and a panic occurs.
> 
> Signed-off-by: Jeffrey Carlyle <jeff.carlyle@...orola.com>
> Signed-off-by: Olusanya Soyannwo <c23746@...orola.com>
> Cc: Tejun Heo <tj@...nel.org>
> Cc: Jens Axboe <jaxboe@...ionio.com>

Supposing it's verified to fix the same issue,

  Acked-by: Tejun Heo <tj@...nel.org>

trivial suggestions below,

> ---
>  lib/scatterlist.c |    8 +++++++-
>  1 files changed, 7 insertions(+), 1 deletions(-)
> 
> diff --git a/lib/scatterlist.c b/lib/scatterlist.c
> index a5ec428..9bc637f 100644
> --- a/lib/scatterlist.c
> +++ b/lib/scatterlist.c
> @@ -248,8 +248,14 @@ int __sg_alloc_table(struct sg_table *table, unsigned int nents,
>  		left -= sg_size;
>  
>  		sg = alloc_fn(alloc_size, gfp_mask);
> -		if (unlikely(!sg))
> +		if (unlikely(!sg)) {
> +			/*
> +			 * Adjust entry count so that proper free function is
> +			 * used in sg_kfree.
> +			 */

I think it would be better why the adjustment is necessary.  IOW,
something like "Adjust entry count to reflect that the last entry of
the previous table won't be used for linkage.  Without this,
sg_kfree() may get confused."

Thanks.

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