linux-kernel - Re: Bug in find_vma_prev

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: <6cafb0f01001311056k3c6a882fla42b714256bb1e6d@mail.gmail.com>
Date:	Sun, 31 Jan 2010 10:56:31 -0800
From:	Tony Perkins <da.perk@...il.com>
To:	Hugh Dickins <hugh.dickins@...cali.co.uk>
Cc:	linux-kernel@...r.kernel.org,
	Andrew Morton <akpm@...ux-foundation.org>,
	"Rafael J. Wysocki" <rjw@...k.pl>, linux-mm@...ck.org
Subject: Re: Bug in find_vma_prev - mmap.c

On Sun, Jan 31, 2010 at 8:25 AM, Hugh Dickins
<hugh.dickins@...cali.co.uk> wrote:
> On Sat, 30 Jan 2010, Rafael J. Wysocki wrote:
>
>> [Adding CCs]
>>
>> On Saturday 30 January 2010, Tony Perkins wrote:
>> > This code returns vma (mm->mmap) if it sees that addr is lower than first VMA.
>> > However, I think it falsely returns vma (mm->mmap) on the case where
>> > addr is in the first VMA.
>> >
>> > If it is the first VMA region:
>> > - *pprev should be set to NULL
>> > - implying prev is NULL
>> > - and should therefore return vma (so in this case, I just added if
>> > it's the first VMA and it's within range)
>> >
>> > /* Same as find_vma, but also return a pointer to the previous VMA in *pprev. */
>> > struct vm_area_struct *
>> > find_vma_prev(struct mm_struct *mm, unsigned long addr,
>> >             struct vm_area_struct **pprev)
>> > {
>> >     struct vm_area_struct *vma = NULL, *prev = NULL;
>> >     struct rb_node *rb_node;
>> >     if (!mm)
>> >         goto out;
>> >
>> >     /* Guard against addr being lower than the first VMA */
>> >     vma = mm->mmap;
>> >
>> >     /* Go through the RB tree quickly. */
>> >     rb_node = mm->mm_rb.rb_node;
>> >
>> >     while (rb_node) {
>> >         struct vm_area_struct *vma_tmp;
>> >         vma_tmp = rb_entry(rb_node, struct vm_area_struct, vm_rb);
>> >
>> >         if (addr < vma_tmp->vm_end) {
>> >             // TONY: if (vma_tmp->vm_start <= addr) vma = vma_tmp; //
>> > this returns the correct 'vma' when vma is the first node (i.e., no
>> > prev)
>> >             rb_node = rb_node->rb_left;
>> >         } else {
>> >             prev = vma_tmp;
>> >             if (!prev->vm_next || (addr < prev->vm_next->vm_end))
>> >                 break;
>> >             rb_node = rb_node->rb_right;
>> >         }
>> >     }
>> >
>> > out:
>> >     *pprev = prev;
>> >     return prev ? prev->vm_next : vma;
>> > }
>> >
>> > Is this a known issue and/or has this problem been addressed?
>> > Also, please CC my email address with responses.
>>
>> Well, I guess you should let the mm people know (CCs added).
>
> Sorry, I don't see what the problem is: I may be misunderstanding.
> Why do you think it is wrong to return the vma which addr is in
> (whether or not that's the first vma)?
>
> find_vma_prev() is supposed to return the same vma as find_vma()
> does, but additionally fill in *pprev.  And find_vma() is supposed
> to return the vma containing or the next vma above the addr supplied.
>
> Hugh
>


Right Hugh,

Say for instance, that addr is not in the list (but is greater than
the last element).
find_vma_prev will return the last node in the list, whereas find_vma
will return NULL.

It seems that it is just inconsistent, in what it should return
regarding the two.
For instance, find_vma_prev will never return NULL, if there's at
least one node within the tree, whereas find_vma would.
find_extend_vma uses find_vma_prev and checks to see if it returns
NULL and is less than the return address (which would always be the
case).

Thanks!

-- 
Aim for Perfection!
--
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/