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: Windows password security audit tool. GUI, reports in PDF.

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

Message-ID: <48f7fe350808011108w2633451bv7e1507b6aa38f9dd@mail.gmail.com>
Date:	Fri, 1 Aug 2008 14:08:57 -0400
From:	"Ryan Hope" <rmh3093@...il.com>
To:	"Hugh Dickins" <hugh@...itas.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

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@...itas.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@...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;
--
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/