linux-kernel - Re: [PATCH][RESEND] mm: fix uninitialized variables for find_vma

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]

Message-ID: <Pine.LNX.4.64.0808011749040.15379@blonde.site>
Date:	Fri, 1 Aug 2008 18:31:37 +0100 (BST)
From:	Hugh Dickins <hugh@...itas.com>
To:	Ryan Hope <rmh3093@...il.com>
cc:	Benny Halevy <bhalevy@...asas.com>,
	LKML <linux-kernel@...r.kernel.org>,
	Andrew Morton <akpm@...ux-foundation.org>
Subject: Re: [PATCH][RESEND] mm: fix uninitialized variables for find_vma_prepare
 callers

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@...il.com>"?

From: Benny Halevy <bhalevy@...asas.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@...asas.com>
Signed-off-by: Hugh Dickins <hugh@...itas.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;