[PATCH] kernel/resource.c: fix stack overflow in __reserve_region_with_split

[PATCH] kernel/resource.c: fix stack overflow in __reserve_region_with_split

[T Makphaibulchoke]

Using recurvise call to try adding a non-conflicting region in the function
__reserve_region_with_split() could result in a stack overflow in the case
that the recursive calls are too deep.  Convert the recursive calls to
an iterative loop to avoid the problem.

Signed-off-by: T Makphaibulchoke <tmac@hp.com>
---
 kernel/resource.c |   32 ++++++++++++++++++--------------
 1 files changed, 18 insertions(+), 14 deletions(-)

diff --git a/kernel/resource.c b/kernel/resource.c
index 34d4588..d6e9f9c 100644
--- a/kernel/resource.c
+++ b/kernel/resource.c
@@ -768,25 +768,29 @@ static void __init __reserve_region_with_split(struct resource *root,
 		return;
 
 	res->name = name;
-	res->start = start;
-	res->end = end;
 	res->flags = IORESOURCE_BUSY;
 
-	conflict = __request_resource(parent, res);
-	if (!conflict)
-		return;
+	while (1) {
+		res->start = start;
+		res->end = end;
 
-	/* failed, split and try again */
-	kfree(res);
+		conflict = __request_resource(parent, res);
+		if (!conflict)
+			break;
 
-	/* conflict covered whole area */
-	if (conflict->start <= start && conflict->end >= end)
-		return;
+		/* conflict covered whole area */
+		if (conflict->start <= start && conflict->end >= end) {
+			kfree(res);
+			break;
+		}
+
+		/* failed, split and try again */
+		if (conflict->start > start)
+			end = conflict->start - 1;
+		if (conflict->end < end)
+			start = conflict->end + 1;
+	}
 
-	if (conflict->start > start)
-		__reserve_region_with_split(root, start, conflict->start-1, name);
-	if (conflict->end < end)
-		__reserve_region_with_split(root, conflict->end+1, end, name);
 }
 
 void __init reserve_region_with_split(struct resource *root,
-- 
1.7.1

Re: [PATCH] kernel/resource.c: fix stack overflow in __reserve_region_with_split

[Ram Pai]

On Mon, Aug 27, 2012 at 06:47:54PM -0600, T Makphaibulchoke wrote:
> Using recurvise call to try adding a non-conflicting region in the function
> __reserve_region_with_split() could result in a stack overflow in the case
> that the recursive calls are too deep.  Convert the recursive calls to
> an iterative loop to avoid the problem.
> 
> Signed-off-by: T Makphaibulchoke <tmac@hp.com>
> ---
>  kernel/resource.c |   32 ++++++++++++++++++--------------
>  1 files changed, 18 insertions(+), 14 deletions(-)
> 
> diff --git a/kernel/resource.c b/kernel/resource.c
> index 34d4588..d6e9f9c 100644
> --- a/kernel/resource.c
> +++ b/kernel/resource.c
> @@ -768,25 +768,29 @@ static void __init __reserve_region_with_split(struct resource *root,
>  		return;
> 
>  	res->name = name;
> -	res->start = start;
> -	res->end = end;
>  	res->flags = IORESOURCE_BUSY;
> 
> -	conflict = __request_resource(parent, res);
> -	if (!conflict)
> -		return;
> +	while (1) {
> +		res->start = start;
> +		res->end = end;
> 
> -	/* failed, split and try again */
> -	kfree(res);
> +		conflict = __request_resource(parent, res);
> +		if (!conflict)
> +			break;
> 
> -	/* conflict covered whole area */
> -	if (conflict->start <= start && conflict->end >= end)
> -		return;
> +		/* conflict covered whole area */
> +		if (conflict->start <= start && conflict->end >= end) {
> +			kfree(res);
> +			break;
> +		}
> +
> +		/* failed, split and try again */
> +		if (conflict->start > start)
> +			end = conflict->start - 1;
> +		if (conflict->end < end)
> +			start = conflict->end + 1;
> +	}

Earlier the patch reserved all areas from 'start' to 'end' skipping any
conflicting intermediate regions.  Your patch will reserve just the
first available fragment before the conflicting range, but will not 
reserve any fragments after the conflicting range. 

For example:
if the region requested is 1 to 100, but 20-30 is already reserved, than
the earlier behavior would reserve 1-20 and 30-100. With your
patch, it will just reserve 1-20.

RP

Re: [PATCH] kernel/resource.c: fix stack overflow in __reserve_region_with_split

[Thavatchai Makphaibulchoke]

On 08/27/2012 10:30 PM, Ram Pai wrote:
> For example:
> if the region requested is 1 to 100, but 20-30 is already reserved, than
> the earlier behavior would reserve 1-20 and 30-100. With your
> patch, it will just reserve 1-20.
> 
> RP
> 

Thanks RP for pointing the problem.  Sorry for missing part of the logic.  I'll fix the problem and resubmit.  Again, thank you very much for your comments.

Thanks,
Mak.