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

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

[Ryan Hope]

This was submitted for 2.6.26-rc8-mm1 but it must have gotten overlooked:

diff --git a/mm/mmap.c b/mm/mmap.c
index 4c5211b..3d65a03 100644
--- a/mm/mmap.c
+++ b/mm/mmap.c
@@ -372,7 +372,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][RESEND] mm: fix uninitialized variables for find_vma_prepare callers

[Ryan Hope]

I forgot to mention that this patch is for 2.6.27-rc1-mm1...

-Ryan

On Fri, Aug 1, 2008 at 11:13 AM, Ryan Hope <rmh3093@gmail.com> wrote:
> This was submitted for 2.6.26-rc8-mm1 but it must have gotten overlooked:
>
> diff --git a/mm/mmap.c b/mm/mmap.c
> index 4c5211b..3d65a03 100644
> --- a/mm/mmap.c
> +++ b/mm/mmap.c
> @@ -372,7 +372,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][RESEND] mm: fix uninitialized variables for find_vma_prepare callers

[Hugh Dickins]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 3775 bytes --]

On Fri, 1 Aug 2008, Ryan Hope wrote:
> This was submitted for 2.6.26-rc8-mm1 but it must have gotten overlooked:

True, thanks, I guess that's my fault: it got stalled by me, because I
found it unsatisfying to fix the uninitialization from a compiler point
of view, whilst leaving it in an undefined state from a usability point
of view.  But since nothing does use those fields in this case, and my
amateurish treephobic experiments didn't find a quick solution to that,
I guess this is an improvement which should go in.

But please, let's credit Benny who posted it, and give his explanation
of what the patch is for, with some additional comment from me.  Which
leaves you out - "Reminded-by: Ryan Hope <rmh3093@gmail.com>"?

From: Benny Halevy <bhalevy@panasas.com>

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

Hugh adds: in fact, none of find_vma_prepare's callers use those values
when a vma is found to be already covering addr, it's either an error
or an occasion to munmap and repeat.  Okay, let's quieten the compiler
(but I would prefer it if pprev, rb_link and rb_parent were meaningful
in that case, rather than whatever's in them from descending the tree).

Signed-off-by: Benny Halevy <bhalevy@panasas.com>
Signed-off-by: Hugh Dickins <hugh@veritas.com>
---

 mm/mmap.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

--- 2.6.27-rc1/mm/mmap.c
+++ linux/mm/mmap.c
@@ -370,7 +370,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][RESEND] mm: fix uninitialized variables for find_vma_prepare callers

[Ryan Hope]

Yeah original author should get credit, i just didnt go back an look,
thanks for doing that.

On Fri, Aug 1, 2008 at 1:31 PM, Hugh Dickins <hugh@veritas.com> wrote:
> On Fri, 1 Aug 2008, Ryan Hope wrote:
>> This was submitted for 2.6.26-rc8-mm1 but it must have gotten overlooked:
>
> True, thanks, I guess that's my fault: it got stalled by me, because I
> found it unsatisfying to fix the uninitialization from a compiler point
> of view, whilst leaving it in an undefined state from a usability point
> of view.  But since nothing does use those fields in this case, and my
> amateurish treephobic experiments didn't find a quick solution to that,
> I guess this is an improvement which should go in.
>
> But please, let's credit Benny who posted it, and give his explanation
> of what the patch is for, with some additional comment from me.  Which
> leaves you out - "Reminded-by: Ryan Hope <rmh3093@gmail.com>"?
>
> From: Benny Halevy <bhalevy@panasas.com>
>
> 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
>
> Hugh adds: in fact, none of find_vma_prepare's callers use those values
> when a vma is found to be already covering addr, it's either an error
> or an occasion to munmap and repeat.  Okay, let's quieten the compiler
> (but I would prefer it if pprev, rb_link and rb_parent were meaningful
> in that case, rather than whatever's in them from descending the tree).
>
> Signed-off-by: Benny Halevy <bhalevy@panasas.com>
> Signed-off-by: Hugh Dickins <hugh@veritas.com>
> ---
>
>  mm/mmap.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
>
> --- 2.6.27-rc1/mm/mmap.c
> +++ linux/mm/mmap.c
> @@ -370,7 +370,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;