[PATCH] mm: fix uninitialized variables for find_vma_prepare callers

[PATCH] mm: fix uninitialized variables for find_vma_prepare callers

[Benny Halevy]

gcc 4.3.0 correctly emits the following warnings.
When a vma covering addr is found, find_vma_prepare indeed returns without
setting pprev, rb_link, and rb_parent.

/usr0/export/dev/bhalevy/git/linux-pnfs-bh-nfs41/mm/mmap.c: In function ‘insert_vm_struct’:
/usr0/export/dev/bhalevy/git/linux-pnfs-bh-nfs41/mm/mmap.c:2085: warning: ‘rb_parent’ may be used uninitialized in this function
/usr0/export/dev/bhalevy/git/linux-pnfs-bh-nfs41/mm/mmap.c:2085: warning: ‘rb_link’ may be used uninitialized in this function
/usr0/export/dev/bhalevy/git/linux-pnfs-bh-nfs41/mm/mmap.c:2084: warning: ‘prev’ may be used uninitialized in this function
/usr0/export/dev/bhalevy/git/linux-pnfs-bh-nfs41/mm/mmap.c: In function ‘copy_vma’:
/usr0/export/dev/bhalevy/git/linux-pnfs-bh-nfs41/mm/mmap.c:2124: warning: ‘rb_parent’ may be used uninitialized in this function
/usr0/export/dev/bhalevy/git/linux-pnfs-bh-nfs41/mm/mmap.c:2124: warning: ‘rb_link’ may be used uninitialized in this function
/usr0/export/dev/bhalevy/git/linux-pnfs-bh-nfs41/mm/mmap.c:2123: warning: ‘prev’ may be used uninitialized in this function
/usr0/export/dev/bhalevy/git/linux-pnfs-bh-nfs41/mm/mmap.c: In function ‘do_brk’:
/usr0/export/dev/bhalevy/git/linux-pnfs-bh-nfs41/mm/mmap.c:1951: warning: ‘rb_parent’ may be used uninitialized in this function
/usr0/export/dev/bhalevy/git/linux-pnfs-bh-nfs41/mm/mmap.c:1951: warning: ‘rb_link’ may be used uninitialized in this function
/usr0/export/dev/bhalevy/git/linux-pnfs-bh-nfs41/mm/mmap.c:1949: warning: ‘prev’ may be used uninitialized in this function
/usr0/export/dev/bhalevy/git/linux-pnfs-bh-nfs41/mm/mmap.c: In function ‘mmap_region’:
/usr0/export/dev/bhalevy/git/linux-pnfs-bh-nfs41/mm/mmap.c:1092: warning: ‘rb_parent’ may be used uninitialized in this function
/usr0/export/dev/bhalevy/git/linux-pnfs-bh-nfs41/mm/mmap.c:1092: warning: ‘rb_link’ may be used uninitialized in this function
/usr0/export/dev/bhalevy/git/linux-pnfs-bh-nfs41/mm/mmap.c:1089: warning: ‘prev’ may be used uninitialized in this function

Signed-off-by: Benny Halevy <bhalevy@panasas.com>
---
 mm/mmap.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/mm/mmap.c b/mm/mmap.c
index 3354fdd..81b9873 100644
--- a/mm/mmap.c
+++ b/mm/mmap.c
@@ -366,7 +366,7 @@ find_vma_prepare(struct mm_struct *mm, unsigned long addr,
 		if (vma_tmp->vm_end > addr) {
 			vma = vma_tmp;
 			if (vma_tmp->vm_start <= addr)
-				return vma;
+				break;
 			__rb_link = &__rb_parent->rb_left;
 		} else {
 			rb_prev = __rb_parent;
-- 
1.5.6.GIT

Re: [PATCH] mm: fix uninitialized variables for find_vma_prepare callers

[Hugh Dickins]

On Mon, 30 Jun 2008, Benny Halevy wrote:
> gcc 4.3.0 correctly emits the following warnings.
> When a vma covering addr is found, find_vma_prepare indeed returns without
> setting pprev, rb_link, and rb_parent.

That's amusing, thank you.

You may wonder how the vma rb_tree has been working all these years
despite that.  The answer is that we only use find_vma_prepare when
about to insert a new vma: if there's anything already there, it's
either an error condition, or we go off and unmap the overlap without
taking any interest in those uninitialized values for linking.

It would be nicer to initialize them, and your patch is certainly
nice and simple.  Would it have the effect, that it returns with
vma == *pprev when addr falls within an existing vma?
That would be a sensible outcome, I think.

Hugh

> [warnings snipped]
> 
> Signed-off-by: Benny Halevy <bhalevy@panasas.com>
> ---
>  mm/mmap.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/mm/mmap.c b/mm/mmap.c
> index 3354fdd..81b9873 100644
> --- a/mm/mmap.c
> +++ b/mm/mmap.c
> @@ -366,7 +366,7 @@ find_vma_prepare(struct mm_struct *mm, unsigned long addr,
>  		if (vma_tmp->vm_end > addr) {
>  			vma = vma_tmp;
>  			if (vma_tmp->vm_start <= addr)
> -				return vma;
> +				break;
>  			__rb_link = &__rb_parent->rb_left;
>  		} else {
>  			rb_prev = __rb_parent;

Re: [PATCH] mm: fix uninitialized variables for find_vma_prepare callers

[Benny Halevy]

On Jun. 30, 2008, 22:00 +0300, Hugh Dickins <hugh@veritas.com> wrote:
> On Mon, 30 Jun 2008, Benny Halevy wrote:
>> gcc 4.3.0 correctly emits the following warnings.
>> When a vma covering addr is found, find_vma_prepare indeed returns without
>> setting pprev, rb_link, and rb_parent.
> 
> That's amusing, thank you.
> 
> You may wonder how the vma rb_tree has been working all these years
> despite that.  The answer is that we only use find_vma_prepare when
> about to insert a new vma: if there's anything already there, it's
> either an error condition, or we go off and unmap the overlap without
> taking any interest in those uninitialized values for linking.
> 
> It would be nicer to initialize them, and your patch is certainly
> nice and simple.  Would it have the effect, that it returns with
> vma == *pprev when addr falls within an existing vma?
> That would be a sensible outcome, I think.

No, *pprev will be set to the last parent encountered with this
patch, but doing what you suggested is possible, though I doubt
it matters since apparently nobody is using pprev in this path
otherwise stuff would've just broken.

Benny

> 
> Hugh
> 
>> [warnings snipped]
>>
>> Signed-off-by: Benny Halevy <bhalevy@panasas.com>
>> ---
>>  mm/mmap.c |    2 +-
>>  1 files changed, 1 insertions(+), 1 deletions(-)
>>
>> diff --git a/mm/mmap.c b/mm/mmap.c
>> index 3354fdd..81b9873 100644
>> --- a/mm/mmap.c
>> +++ b/mm/mmap.c
>> @@ -366,7 +366,7 @@ find_vma_prepare(struct mm_struct *mm, unsigned long addr,
>>  		if (vma_tmp->vm_end > addr) {
>>  			vma = vma_tmp;
>>  			if (vma_tmp->vm_start <= addr)
>> -				return vma;
>> +				break;
>>  			__rb_link = &__rb_parent->rb_left;
>>  		} else {
>>  			rb_prev = __rb_parent;