Re: mm: NULL ptr deref in anon_vma_interval_tree_verify

2012-11-07 Thread Bob Liu 
Hi Michel,

On Wed, Nov 7, 2012 at 11:54 AM, Michel Lespinasse  wrote:
> On Tue, Nov 6, 2012 at 12:24 AM, Michel Lespinasse  wrote:
>> On Mon, Nov 5, 2012 at 5:41 AM, Michel Lespinasse  wrote:
>>> On Sun, Nov 4, 2012 at 8:44 PM, Michel Lespinasse  wrote:
 On Sun, Nov 4, 2012 at 8:14 PM, Bob Liu  wrote:
> Hmm, I attached a simple fix patch.

 Reviewed-by: Michel Lespinasse 
 (also ran some tests with it, but I could never reproduce the original
 issue anyway).
>>>
>>> Wait a minute, this is actually wrong. You need to call
>>> vma_lock_anon_vma() / vma_unlock_anon_vma() to avoid the issue with
>>> vma->anon_vma == NULL.
>>>
>>> I'll fix it and integrate it into my next patch series, which I intend
>>> to send later today. (I am adding new code into validate_mm(), so that
>>> it's easier to have it in the same patch series to avoid merge
>>> conflicts)
>>
>> Hmmm, now I'm getting confused about anon_vma locking again :/
>>
>> As Hugh privately remarked to me, the same_vma linked list is supposed
>> to be protected by exclusive mmap_sem ownership, not by anon_vma lock.
>> So now looking at it a bit more, I'm not sure what race we're
>> preventing by taking the anon_vma lock in validate_mm() ???
>
> Looking at it a bit more:
>
> the same_vma linked list is *generally* protected by *exclusive*
> mmap_sem ownership. However, in expand_stack() we only have *shared*
> mmap_sem ownership, so that two concurrent expand_stack() calls
> (possibly on different vmas that have a different anon_vma lock) could
> race with each other. For this reason we do need the validate_mm()
> taking each vma's anon_vma lock (if any) before calling
> anon_vma_interval_tree_verify().
>

Sorry for the late response.
Actually my origin concern was:
avc was removed in some race place which caused the NULL pointer deref
in validate_mm().

But after looking it more, i didn't find out the race place.
I think avc only freed at free_pgtable() --> unlink_anon_vmas().

> While this justifies Bob's patch, this does not explain Sasha's
> reports - in both of them the backtrace did not involve
> expand_stack(), and there should be exclusive mmap_sem ownership, so
> I'm still unclear as to what could be causing Sasha's issue.
>
> Sasha, how reproduceable is this ?
>
> Also, would the following change print something when the issue triggers ?
>
> diff --git a/mm/mmap.c b/mm/mmap.c
> index 619b280505fe..4c09e7ebcfa7 100644
> --- a/mm/mmap.c
> +++ b/mm/mmap.c
> @@ -404,8 +404,13 @@ void validate_mm(struct mm_struct *mm)
> while (vma) {
> struct anon_vma_chain *avc;
> vma_lock_anon_vma(vma);

And for our patch, i think vma_lock_anon_vma()/anon_vma_lock() is used
to protect
the same_anon_vma list.
It seems not suitable here.

> -   list_for_each_entry(avc, >anon_vma_chain, same_vma)
> +   list_for_each_entry(avc, >anon_vma_chain, same_vma) {
> +   if (avc->vma != vma) {
> +   printk("avc->vma %p vma %p\n", avc->vma, vma);
> +   bug = 1;
> +   }
> anon_vma_interval_tree_verify(avc);
> +   }
> vma_unlock_anon_vma(vma);
> highest_address = vma->vm_end;
> vma = vma->vm_next;
>
> --
> Michel "Walken" Lespinasse
> A program is never fully debugged until the last user dies.

-- 
Thanks,
--Bob
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: mm: NULL ptr deref in anon_vma_interval_tree_verify

2012-11-07 Thread Bob Liu 
Hi Michel,

On Wed, Nov 7, 2012 at 11:54 AM, Michel Lespinasse wal...@google.com wrote:
 On Tue, Nov 6, 2012 at 12:24 AM, Michel Lespinasse wal...@google.com wrote:
 On Mon, Nov 5, 2012 at 5:41 AM, Michel Lespinasse wal...@google.com wrote:
 On Sun, Nov 4, 2012 at 8:44 PM, Michel Lespinasse wal...@google.com wrote:
 On Sun, Nov 4, 2012 at 8:14 PM, Bob Liu lliu...@gmail.com wrote:
 Hmm, I attached a simple fix patch.

 Reviewed-by: Michel Lespinasse wal...@google.com
 (also ran some tests with it, but I could never reproduce the original
 issue anyway).

 Wait a minute, this is actually wrong. You need to call
 vma_lock_anon_vma() / vma_unlock_anon_vma() to avoid the issue with
 vma-anon_vma == NULL.

 I'll fix it and integrate it into my next patch series, which I intend
 to send later today. (I am adding new code into validate_mm(), so that
 it's easier to have it in the same patch series to avoid merge
 conflicts)

 Hmmm, now I'm getting confused about anon_vma locking again :/

 As Hugh privately remarked to me, the same_vma linked list is supposed
 to be protected by exclusive mmap_sem ownership, not by anon_vma lock.
 So now looking at it a bit more, I'm not sure what race we're
 preventing by taking the anon_vma lock in validate_mm() ???

 Looking at it a bit more:

 the same_vma linked list is *generally* protected by *exclusive*
 mmap_sem ownership. However, in expand_stack() we only have *shared*
 mmap_sem ownership, so that two concurrent expand_stack() calls
 (possibly on different vmas that have a different anon_vma lock) could
 race with each other. For this reason we do need the validate_mm()
 taking each vma's anon_vma lock (if any) before calling
 anon_vma_interval_tree_verify().


Sorry for the late response.
Actually my origin concern was:
avc was removed in some race place which caused the NULL pointer deref
in validate_mm().

But after looking it more, i didn't find out the race place.
I think avc only freed at free_pgtable() -- unlink_anon_vmas().

 While this justifies Bob's patch, this does not explain Sasha's
 reports - in both of them the backtrace did not involve
 expand_stack(), and there should be exclusive mmap_sem ownership, so
 I'm still unclear as to what could be causing Sasha's issue.

 Sasha, how reproduceable is this ?

 Also, would the following change print something when the issue triggers ?

 diff --git a/mm/mmap.c b/mm/mmap.c
 index 619b280505fe..4c09e7ebcfa7 100644
 --- a/mm/mmap.c
 +++ b/mm/mmap.c
 @@ -404,8 +404,13 @@ void validate_mm(struct mm_struct *mm)
 while (vma) {
 struct anon_vma_chain *avc;
 vma_lock_anon_vma(vma);

And for our patch, i think vma_lock_anon_vma()/anon_vma_lock() is used
to protect
the same_anon_vma list.
It seems not suitable here.

 -   list_for_each_entry(avc, vma-anon_vma_chain, same_vma)
 +   list_for_each_entry(avc, vma-anon_vma_chain, same_vma) {
 +   if (avc-vma != vma) {
 +   printk(avc-vma %p vma %p\n, avc-vma, vma);
 +   bug = 1;
 +   }
 anon_vma_interval_tree_verify(avc);
 +   }
 vma_unlock_anon_vma(vma);
 highest_address = vma-vm_end;
 vma = vma-vm_next;

 --
 Michel Walken Lespinasse
 A program is never fully debugged until the last user dies.

-- 
Thanks,
--Bob
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: mm: NULL ptr deref in anon_vma_interval_tree_verify

2012-11-06 Thread Sasha Levin 
On 11/06/2012 10:54 PM, Michel Lespinasse wrote:
> On Tue, Nov 6, 2012 at 12:24 AM, Michel Lespinasse  wrote:
>> On Mon, Nov 5, 2012 at 5:41 AM, Michel Lespinasse  wrote:
>>> On Sun, Nov 4, 2012 at 8:44 PM, Michel Lespinasse  wrote:
 On Sun, Nov 4, 2012 at 8:14 PM, Bob Liu  wrote:
> Hmm, I attached a simple fix patch.

 Reviewed-by: Michel Lespinasse 
 (also ran some tests with it, but I could never reproduce the original
 issue anyway).
>>>
>>> Wait a minute, this is actually wrong. You need to call
>>> vma_lock_anon_vma() / vma_unlock_anon_vma() to avoid the issue with
>>> vma->anon_vma == NULL.
>>>
>>> I'll fix it and integrate it into my next patch series, which I intend
>>> to send later today. (I am adding new code into validate_mm(), so that
>>> it's easier to have it in the same patch series to avoid merge
>>> conflicts)
>>
>> Hmmm, now I'm getting confused about anon_vma locking again :/
>>
>> As Hugh privately remarked to me, the same_vma linked list is supposed
>> to be protected by exclusive mmap_sem ownership, not by anon_vma lock.
>> So now looking at it a bit more, I'm not sure what race we're
>> preventing by taking the anon_vma lock in validate_mm() ???
> 
> Looking at it a bit more:
> 
> the same_vma linked list is *generally* protected by *exclusive*
> mmap_sem ownership. However, in expand_stack() we only have *shared*
> mmap_sem ownership, so that two concurrent expand_stack() calls
> (possibly on different vmas that have a different anon_vma lock) could
> race with each other. For this reason we do need the validate_mm()
> taking each vma's anon_vma lock (if any) before calling
> anon_vma_interval_tree_verify().
> 
> While this justifies Bob's patch, this does not explain Sasha's
> reports - in both of them the backtrace did not involve
> expand_stack(), and there should be exclusive mmap_sem ownership, so
> I'm still unclear as to what could be causing Sasha's issue.
> 
> Sasha, how reproduceable is this ?

This is pretty hard to reproduce, I've seen this only twice so far.

> 
> Also, would the following change print something when the issue triggers ?

I'll run it with your patch, but as I've mentioned above - it's a PITA
to reproduce.


Thanks,
Sasha
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: mm: NULL ptr deref in anon_vma_interval_tree_verify

2012-11-06 Thread Michel Lespinasse 
On Tue, Nov 6, 2012 at 12:24 AM, Michel Lespinasse  wrote:
> On Mon, Nov 5, 2012 at 5:41 AM, Michel Lespinasse  wrote:
>> On Sun, Nov 4, 2012 at 8:44 PM, Michel Lespinasse  wrote:
>>> On Sun, Nov 4, 2012 at 8:14 PM, Bob Liu  wrote:
 Hmm, I attached a simple fix patch.
>>>
>>> Reviewed-by: Michel Lespinasse 
>>> (also ran some tests with it, but I could never reproduce the original
>>> issue anyway).
>>
>> Wait a minute, this is actually wrong. You need to call
>> vma_lock_anon_vma() / vma_unlock_anon_vma() to avoid the issue with
>> vma->anon_vma == NULL.
>>
>> I'll fix it and integrate it into my next patch series, which I intend
>> to send later today. (I am adding new code into validate_mm(), so that
>> it's easier to have it in the same patch series to avoid merge
>> conflicts)
>
> Hmmm, now I'm getting confused about anon_vma locking again :/
>
> As Hugh privately remarked to me, the same_vma linked list is supposed
> to be protected by exclusive mmap_sem ownership, not by anon_vma lock.
> So now looking at it a bit more, I'm not sure what race we're
> preventing by taking the anon_vma lock in validate_mm() ???

Looking at it a bit more:

the same_vma linked list is *generally* protected by *exclusive*
mmap_sem ownership. However, in expand_stack() we only have *shared*
mmap_sem ownership, so that two concurrent expand_stack() calls
(possibly on different vmas that have a different anon_vma lock) could
race with each other. For this reason we do need the validate_mm()
taking each vma's anon_vma lock (if any) before calling
anon_vma_interval_tree_verify().

While this justifies Bob's patch, this does not explain Sasha's
reports - in both of them the backtrace did not involve
expand_stack(), and there should be exclusive mmap_sem ownership, so
I'm still unclear as to what could be causing Sasha's issue.

Sasha, how reproduceable is this ?

Also, would the following change print something when the issue triggers ?

diff --git a/mm/mmap.c b/mm/mmap.c
index 619b280505fe..4c09e7ebcfa7 100644
--- a/mm/mmap.c
+++ b/mm/mmap.c
@@ -404,8 +404,13 @@ void validate_mm(struct mm_struct *mm)
while (vma) {
struct anon_vma_chain *avc;
vma_lock_anon_vma(vma);
-   list_for_each_entry(avc, >anon_vma_chain, same_vma)
+   list_for_each_entry(avc, >anon_vma_chain, same_vma) {
+   if (avc->vma != vma) {
+   printk("avc->vma %p vma %p\n", avc->vma, vma);
+   bug = 1;
+   }
anon_vma_interval_tree_verify(avc);
+   }
vma_unlock_anon_vma(vma);
highest_address = vma->vm_end;
vma = vma->vm_next;

-- 
Michel "Walken" Lespinasse
A program is never fully debugged until the last user dies.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: mm: NULL ptr deref in anon_vma_interval_tree_verify

2012-11-06 Thread Michel Lespinasse 
On Mon, Nov 5, 2012 at 5:41 AM, Michel Lespinasse  wrote:
> On Sun, Nov 4, 2012 at 8:44 PM, Michel Lespinasse  wrote:
>> On Sun, Nov 4, 2012 at 8:14 PM, Bob Liu  wrote:
>>> Hmm, I attached a simple fix patch.
>>
>> Reviewed-by: Michel Lespinasse 
>> (also ran some tests with it, but I could never reproduce the original
>> issue anyway).
>
> Wait a minute, this is actually wrong. You need to call
> vma_lock_anon_vma() / vma_unlock_anon_vma() to avoid the issue with
> vma->anon_vma == NULL.
>
> I'll fix it and integrate it into my next patch series, which I intend
> to send later today. (I am adding new code into validate_mm(), so that
> it's easier to have it in the same patch series to avoid merge
> conflicts)

Hmmm, now I'm getting confused about anon_vma locking again :/

As Hugh privately remarked to me, the same_vma linked list is supposed
to be protected by exclusive mmap_sem ownership, not by anon_vma lock.
So now looking at it a bit more, I'm not sure what race we're
preventing by taking the anon_vma lock in validate_mm() ???

-- 
Michel "Walken" Lespinasse
A program is never fully debugged until the last user dies.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: mm: NULL ptr deref in anon_vma_interval_tree_verify

2012-11-06 Thread Michel Lespinasse 
On Mon, Nov 5, 2012 at 5:41 AM, Michel Lespinasse wal...@google.com wrote:
 On Sun, Nov 4, 2012 at 8:44 PM, Michel Lespinasse wal...@google.com wrote:
 On Sun, Nov 4, 2012 at 8:14 PM, Bob Liu lliu...@gmail.com wrote:
 Hmm, I attached a simple fix patch.

 Reviewed-by: Michel Lespinasse wal...@google.com
 (also ran some tests with it, but I could never reproduce the original
 issue anyway).

 Wait a minute, this is actually wrong. You need to call
 vma_lock_anon_vma() / vma_unlock_anon_vma() to avoid the issue with
 vma-anon_vma == NULL.

 I'll fix it and integrate it into my next patch series, which I intend
 to send later today. (I am adding new code into validate_mm(), so that
 it's easier to have it in the same patch series to avoid merge
 conflicts)

Hmmm, now I'm getting confused about anon_vma locking again :/

As Hugh privately remarked to me, the same_vma linked list is supposed
to be protected by exclusive mmap_sem ownership, not by anon_vma lock.
So now looking at it a bit more, I'm not sure what race we're
preventing by taking the anon_vma lock in validate_mm() ???

-- 
Michel Walken Lespinasse
A program is never fully debugged until the last user dies.
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: mm: NULL ptr deref in anon_vma_interval_tree_verify

2012-11-06 Thread Michel Lespinasse 
On Tue, Nov 6, 2012 at 12:24 AM, Michel Lespinasse wal...@google.com wrote:
 On Mon, Nov 5, 2012 at 5:41 AM, Michel Lespinasse wal...@google.com wrote:
 On Sun, Nov 4, 2012 at 8:44 PM, Michel Lespinasse wal...@google.com wrote:
 On Sun, Nov 4, 2012 at 8:14 PM, Bob Liu lliu...@gmail.com wrote:
 Hmm, I attached a simple fix patch.

 Reviewed-by: Michel Lespinasse wal...@google.com
 (also ran some tests with it, but I could never reproduce the original
 issue anyway).

 Wait a minute, this is actually wrong. You need to call
 vma_lock_anon_vma() / vma_unlock_anon_vma() to avoid the issue with
 vma-anon_vma == NULL.

 I'll fix it and integrate it into my next patch series, which I intend
 to send later today. (I am adding new code into validate_mm(), so that
 it's easier to have it in the same patch series to avoid merge
 conflicts)

 Hmmm, now I'm getting confused about anon_vma locking again :/

 As Hugh privately remarked to me, the same_vma linked list is supposed
 to be protected by exclusive mmap_sem ownership, not by anon_vma lock.
 So now looking at it a bit more, I'm not sure what race we're
 preventing by taking the anon_vma lock in validate_mm() ???

Looking at it a bit more:

the same_vma linked list is *generally* protected by *exclusive*
mmap_sem ownership. However, in expand_stack() we only have *shared*
mmap_sem ownership, so that two concurrent expand_stack() calls
(possibly on different vmas that have a different anon_vma lock) could
race with each other. For this reason we do need the validate_mm()
taking each vma's anon_vma lock (if any) before calling
anon_vma_interval_tree_verify().

While this justifies Bob's patch, this does not explain Sasha's
reports - in both of them the backtrace did not involve
expand_stack(), and there should be exclusive mmap_sem ownership, so
I'm still unclear as to what could be causing Sasha's issue.

Sasha, how reproduceable is this ?

Also, would the following change print something when the issue triggers ?

diff --git a/mm/mmap.c b/mm/mmap.c
index 619b280505fe..4c09e7ebcfa7 100644
--- a/mm/mmap.c
+++ b/mm/mmap.c
@@ -404,8 +404,13 @@ void validate_mm(struct mm_struct *mm)
while (vma) {
struct anon_vma_chain *avc;
vma_lock_anon_vma(vma);
-   list_for_each_entry(avc, vma-anon_vma_chain, same_vma)
+   list_for_each_entry(avc, vma-anon_vma_chain, same_vma) {
+   if (avc-vma != vma) {
+   printk(avc-vma %p vma %p\n, avc-vma, vma);
+   bug = 1;
+   }
anon_vma_interval_tree_verify(avc);
+   }
vma_unlock_anon_vma(vma);
highest_address = vma-vm_end;
vma = vma-vm_next;

-- 
Michel Walken Lespinasse
A program is never fully debugged until the last user dies.
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: mm: NULL ptr deref in anon_vma_interval_tree_verify

2012-11-06 Thread Sasha Levin 
On 11/06/2012 10:54 PM, Michel Lespinasse wrote:
 On Tue, Nov 6, 2012 at 12:24 AM, Michel Lespinasse wal...@google.com wrote:
 On Mon, Nov 5, 2012 at 5:41 AM, Michel Lespinasse wal...@google.com wrote:
 On Sun, Nov 4, 2012 at 8:44 PM, Michel Lespinasse wal...@google.com wrote:
 On Sun, Nov 4, 2012 at 8:14 PM, Bob Liu lliu...@gmail.com wrote:
 Hmm, I attached a simple fix patch.

 Reviewed-by: Michel Lespinasse wal...@google.com
 (also ran some tests with it, but I could never reproduce the original
 issue anyway).

 Wait a minute, this is actually wrong. You need to call
 vma_lock_anon_vma() / vma_unlock_anon_vma() to avoid the issue with
 vma-anon_vma == NULL.

 I'll fix it and integrate it into my next patch series, which I intend
 to send later today. (I am adding new code into validate_mm(), so that
 it's easier to have it in the same patch series to avoid merge
 conflicts)

 Hmmm, now I'm getting confused about anon_vma locking again :/

 As Hugh privately remarked to me, the same_vma linked list is supposed
 to be protected by exclusive mmap_sem ownership, not by anon_vma lock.
 So now looking at it a bit more, I'm not sure what race we're
 preventing by taking the anon_vma lock in validate_mm() ???
 
 Looking at it a bit more:
 
 the same_vma linked list is *generally* protected by *exclusive*
 mmap_sem ownership. However, in expand_stack() we only have *shared*
 mmap_sem ownership, so that two concurrent expand_stack() calls
 (possibly on different vmas that have a different anon_vma lock) could
 race with each other. For this reason we do need the validate_mm()
 taking each vma's anon_vma lock (if any) before calling
 anon_vma_interval_tree_verify().
 
 While this justifies Bob's patch, this does not explain Sasha's
 reports - in both of them the backtrace did not involve
 expand_stack(), and there should be exclusive mmap_sem ownership, so
 I'm still unclear as to what could be causing Sasha's issue.
 
 Sasha, how reproduceable is this ?

This is pretty hard to reproduce, I've seen this only twice so far.

 
 Also, would the following change print something when the issue triggers ?

I'll run it with your patch, but as I've mentioned above - it's a PITA
to reproduce.


Thanks,
Sasha
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: mm: NULL ptr deref in anon_vma_interval_tree_verify

2012-11-05 Thread Michel Lespinasse 
On Sun, Nov 4, 2012 at 8:44 PM, Michel Lespinasse  wrote:
> On Sun, Nov 4, 2012 at 8:14 PM, Bob Liu  wrote:
>> Hmm, I attached a simple fix patch.
>
> Reviewed-by: Michel Lespinasse 
> (also ran some tests with it, but I could never reproduce the original
> issue anyway).

Wait a minute, this is actually wrong. You need to call
vma_lock_anon_vma() / vma_unlock_anon_vma() to avoid the issue with
vma->anon_vma == NULL.

I'll fix it and integrate it into my next patch series, which I intend
to send later today. (I am adding new code into validate_mm(), so that
it's easier to have it in the same patch series to avoid merge
conflicts)

-- 
Michel "Walken" Lespinasse
A program is never fully debugged until the last user dies.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: mm: NULL ptr deref in anon_vma_interval_tree_verify

2012-11-05 Thread Michel Lespinasse 
On Sun, Nov 4, 2012 at 8:44 PM, Michel Lespinasse wal...@google.com wrote:
 On Sun, Nov 4, 2012 at 8:14 PM, Bob Liu lliu...@gmail.com wrote:
 Hmm, I attached a simple fix patch.

 Reviewed-by: Michel Lespinasse wal...@google.com
 (also ran some tests with it, but I could never reproduce the original
 issue anyway).

Wait a minute, this is actually wrong. You need to call
vma_lock_anon_vma() / vma_unlock_anon_vma() to avoid the issue with
vma-anon_vma == NULL.

I'll fix it and integrate it into my next patch series, which I intend
to send later today. (I am adding new code into validate_mm(), so that
it's easier to have it in the same patch series to avoid merge
conflicts)

-- 
Michel Walken Lespinasse
A program is never fully debugged until the last user dies.
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: mm: NULL ptr deref in anon_vma_interval_tree_verify

2012-11-04 Thread Michel Lespinasse 
On Sun, Nov 4, 2012 at 8:14 PM, Bob Liu  wrote:
> Hmm, I attached a simple fix patch.

Reviewed-by: Michel Lespinasse 
(also ran some tests with it, but I could never reproduce the original
issue anyway).

Bob, it would be easier if you had sent the original patch inline
rather than as an attachment :)

Andrew, can you get this simple fix into your -mm tree ?

-- 
Michel "Walken" Lespinasse
A program is never fully debugged until the last user dies.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: mm: NULL ptr deref in anon_vma_interval_tree_verify

2012-11-04 Thread Bob Liu 
On Mon, Nov 5, 2012 at 11:31 AM, Michel Lespinasse  wrote:
> On Sun, Nov 4, 2012 at 6:20 PM, Bob Liu  wrote:
>> The loop for each entry of vma->anon_vma_chain in validate_mm() is not
>> protected by anon_vma lock.
>> I think that may be the cause.
>>
>> Michel, What's your opinion？
>
> Good catch, I think that's it. Somehow it had not occured to me to

Hmm, I attached a simple fix patch.
Sasha,
Could you have a test to see whether it can fix your issue?

Thanks,
-Bob


0001-mm-add-anon_vma_lock-to-validate_mm.patch
Description: Binary data

Re: mm: NULL ptr deref in anon_vma_interval_tree_verify

2012-11-04 Thread Michel Lespinasse 
On Sun, Nov 4, 2012 at 6:20 PM, Bob Liu  wrote:
> The loop for each entry of vma->anon_vma_chain in validate_mm() is not
> protected by anon_vma lock.
> I think that may be the cause.
>
> Michel, What's your opinion？

Good catch, I think that's it. Somehow it had not occured to me to
verify the checker code - as in, who's checking the checker ? :)

-- 
Michel "Walken" Lespinasse
A program is never fully debugged until the last user dies.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: mm: NULL ptr deref in anon_vma_interval_tree_verify

2012-11-04 Thread Bob Liu 
On Sat, Nov 3, 2012 at 11:51 AM, Sasha Levin  wrote:
> Ping?
>
> On Thu, Oct 25, 2012 at 4:26 PM, Sasha Levin  wrote:
>> On 10/18/2012 06:46 PM, Sasha Levin wrote:
>>> Hi all,
>>>
>>> While fuzzing with trinity inside a KVM tools (lkvm) guest, on today's 
>>> linux-next kernel,
>>> I saw the following:
>>>
>>> [ 1857.278176] BUG: unable to handle kernel NULL pointer dereference at 
>>> 0090
>>> [ 1857.283725] IP: [] 
>>> anon_vma_interval_tree_verify+0xf/0xa0
>>> [ 1857.283725] PGD 6e19e067 PUD 6e19f067 PMD 0
>>> [ 1857.283725] Oops:  [#1] PREEMPT SMP DEBUG_PAGEALLOC
>>> [ 1857.283725] Dumping ftrace buffer:
>>> [ 1857.283725](ftrace buffer empty)
>>> [ 1857.283725] CPU 2
>>> [ 1857.283725] Pid: 15637, comm: trinity-child18 Tainted: GW
>>> 3.7.0-rc1-next-20121018-sasha-2-g60a870d-dirty #61
>>> [ 1857.283725] RIP: 0010:[]  [] 
>>> anon_vma_interval_tree_verify+0xf/0xa0
>>> [ 1857.283725] RSP: 0018:88007df0fce8  EFLAGS: 00010296
>>> [ 1857.283725] RAX: 880089db1000 RBX: 880089db0ff0 RCX: 
>>> 8800869e6928
>>> [ 1857.283725] RDX:  RSI: 880089db1008 RDI: 
>>> 880089db0ff0
>>> [ 1857.283725] RBP: 88007df0fcf8 R08: 88006427d508 R09: 
>>> 88012bb95f20
>>> [ 1857.283725] R10: 0001 R11: 8800c8525c60 R12: 
>>> 88006e199370
>>> [ 1857.283725] R13: 88006e199300 R14:  R15: 
>>> 880089db1000
>>> [ 1857.283725] FS:  7f322fd4c700() GS:88004d60() 
>>> knlGS:
>>> [ 1857.283725] CS:  0010 DS:  ES:  CR0: 80050033
>>> [ 1857.283725] CR2: 0090 CR3: 6e19d000 CR4: 
>>> 000406e0
>>> [ 1857.283725] DR0:  DR1:  DR2: 
>>> 
>>> [ 1857.283725] DR3:  DR6: 0ff0 DR7: 
>>> 0400
>>> [ 1857.283725] Process trinity-child18 (pid: 15637, threadinfo 
>>> 88007df0e000, task 88007ac8)
>>> [ 1857.283725] Stack:
>>> [ 1857.283725]  88007df0fd38 880089db0ff0 88007df0fd48 
>>> 81233b58
>>> [ 1857.283725]  88007df0fd38 880089db1000 80d0 
>>> 880089db1000
>>> [ 1857.283725]  88012bb95f20 885d97c8 885d97d8 
>>> 880089db1000
>>> [ 1857.283725] Call Trace:
>>> [ 1857.283725]  [] validate_mm+0x58/0x1e0
>>> [ 1857.283725]  [] vma_link+0x94/0xe0
>>> [ 1857.283725]  [] ? _raw_spin_unlock_irqrestore+0x84/0xb0
>>> [ 1857.283725]  [] mmap_region+0x3f5/0x5c0
>>> [ 1857.283725]  [] do_mmap_pgoff+0x2b7/0x330
>>> [ 1857.283725]  [] ? vm_mmap_pgoff+0x61/0xa0
>>> [ 1857.283725]  [] vm_mmap_pgoff+0x7a/0xa0
>>> [ 1857.283725]  [] sys_mmap_pgoff+0x182/0x1a0
>>> [ 1857.283725]  [] ? syscall_trace_enter+0x20/0x2e0
>>> [ 1857.283725]  [] sys_mmap+0x1d/0x20
>>> [ 1857.283725]  [] tracesys+0xe1/0xe6
>>> [ 1857.283725] Code: 48 39 ce 77 9e f3 c3 0f 1f 44 00 00 31 c0 c3 66 66 66 
>>> 66 2e 0f 1f 84 00 00 00 00 00 55 48 89 e5 53 48 89 fb
>>> 48 83 ec 08 48 8b 17 <48> 8b 8a 90 00 00 00 48 39 4f 40 74 34 80 3d a6 82 
>>> 5b 04 00 75
>>> [ 1857.283725] RIP  [] 
>>> anon_vma_interval_tree_verify+0xf/0xa0
>>> [ 1857.283725]  RSP 
>>> [ 1857.283725] CR2: 0090
>>> [ 1858.611277] ---[ end trace b51cc425e9b07fc0 ]---
>>>
>>> The obvious part is that anon_vma_interval_tree_verify() got called with 
>>> node == NULL, but when
>>> looking at the caller:
>>>
>>> list_for_each_entry(avc, >anon_vma_chain, same_vma)
>>> anon_vma_interval_tree_verify(avc);
>>>
>>> How it got called with said NULL becomes less obvious.
>>
>> I've hit a similar one with today's -next. It isn't exactly the same, but
>> I suspect it's the same issue.
>>
>> [ 1523.657950] BUG: unable to handle kernel paging request at 
>> fff0
>> [ 1523.660022] IP: [] 
>> anon_vma_interval_tree_verify+0xc/0xa0
>> [ 1523.660022] PGD 4e28067 PUD 4e29067 PMD 0
>> [ 1523.675725] Oops:  [#1] PREEMPT SMP DEBUG_PAGEALLOC
>> [ 1523.750066] CPU 0
>> [ 1523.750066] Pid: 9050, comm: trinity-child64 Tainted: GW
>> 3.7.0-rc2-next-20121025-sasha-1-g673f98e-dirty #77
>> [ 1523.750066] RIP: 0010:[]  [] 
>> anon_vma_interval_tree_verify+0xc/0xa0
>> [ 1523.750066] RSP: 0018:880045f81d48  EFLAGS: 00010296
>> [ 1523.750066] RAX:  RBX: fff0 RCX: 
>> 
>> [ 1523.750066] RDX:  RSI: 0001 RDI: 
>> fff0
>> [ 1523.750066] RBP: 880045f81d58 R08:  R09: 
>> 0f14
>> [ 1523.750066] R10: 0f12 R11:  R12: 
>> 8800096c8d70
>> [ 1523.750066] R13: 8800096c8d00 R14:  R15: 
>> 8800095b45e0
>> [ 1523.750066] FS:  7f7a923f3700() GS:88001360() 
>> knlGS:
>> [ 1523.750066] CS:  0010 DS:  ES:  CR0: 80050033
>> [ 1523.750066] CR2: fff0 CR3: 0969d000 CR4: 
>> 000406f0
>> [

Re: mm: NULL ptr deref in anon_vma_interval_tree_verify

2012-11-04 Thread Bob Liu 
On Sat, Nov 3, 2012 at 11:51 AM, Sasha Levin levinsasha...@gmail.com wrote:
 Ping?

 On Thu, Oct 25, 2012 at 4:26 PM, Sasha Levin levinsasha...@gmail.com wrote:
 On 10/18/2012 06:46 PM, Sasha Levin wrote:
 Hi all,

 While fuzzing with trinity inside a KVM tools (lkvm) guest, on today's 
 linux-next kernel,
 I saw the following:

 [ 1857.278176] BUG: unable to handle kernel NULL pointer dereference at 
 0090
 [ 1857.283725] IP: [81229d0f] 
 anon_vma_interval_tree_verify+0xf/0xa0
 [ 1857.283725] PGD 6e19e067 PUD 6e19f067 PMD 0
 [ 1857.283725] Oops:  [#1] PREEMPT SMP DEBUG_PAGEALLOC
 [ 1857.283725] Dumping ftrace buffer:
 [ 1857.283725](ftrace buffer empty)
 [ 1857.283725] CPU 2
 [ 1857.283725] Pid: 15637, comm: trinity-child18 Tainted: GW
 3.7.0-rc1-next-20121018-sasha-2-g60a870d-dirty #61
 [ 1857.283725] RIP: 0010:[81229d0f]  [81229d0f] 
 anon_vma_interval_tree_verify+0xf/0xa0
 [ 1857.283725] RSP: 0018:88007df0fce8  EFLAGS: 00010296
 [ 1857.283725] RAX: 880089db1000 RBX: 880089db0ff0 RCX: 
 8800869e6928
 [ 1857.283725] RDX:  RSI: 880089db1008 RDI: 
 880089db0ff0
 [ 1857.283725] RBP: 88007df0fcf8 R08: 88006427d508 R09: 
 88012bb95f20
 [ 1857.283725] R10: 0001 R11: 8800c8525c60 R12: 
 88006e199370
 [ 1857.283725] R13: 88006e199300 R14:  R15: 
 880089db1000
 [ 1857.283725] FS:  7f322fd4c700() GS:88004d60() 
 knlGS:
 [ 1857.283725] CS:  0010 DS:  ES:  CR0: 80050033
 [ 1857.283725] CR2: 0090 CR3: 6e19d000 CR4: 
 000406e0
 [ 1857.283725] DR0:  DR1:  DR2: 
 
 [ 1857.283725] DR3:  DR6: 0ff0 DR7: 
 0400
 [ 1857.283725] Process trinity-child18 (pid: 15637, threadinfo 
 88007df0e000, task 88007ac8)
 [ 1857.283725] Stack:
 [ 1857.283725]  88007df0fd38 880089db0ff0 88007df0fd48 
 81233b58
 [ 1857.283725]  88007df0fd38 880089db1000 80d0 
 880089db1000
 [ 1857.283725]  88012bb95f20 885d97c8 885d97d8 
 880089db1000
 [ 1857.283725] Call Trace:
 [ 1857.283725]  [81233b58] validate_mm+0x58/0x1e0
 [ 1857.283725]  [81233da4] vma_link+0x94/0xe0
 [ 1857.283725]  [83a67fd4] ? _raw_spin_unlock_irqrestore+0x84/0xb0
 [ 1857.283725]  [81235f75] mmap_region+0x3f5/0x5c0
 [ 1857.283725]  [812363f7] do_mmap_pgoff+0x2b7/0x330
 [ 1857.283725]  [81220fd1] ? vm_mmap_pgoff+0x61/0xa0
 [ 1857.283725]  [81220fea] vm_mmap_pgoff+0x7a/0xa0
 [ 1857.283725]  [81234c72] sys_mmap_pgoff+0x182/0x1a0
 [ 1857.283725]  [8107dc40] ? syscall_trace_enter+0x20/0x2e0
 [ 1857.283725]  [810738dd] sys_mmap+0x1d/0x20
 [ 1857.283725]  [83a69ad8] tracesys+0xe1/0xe6
 [ 1857.283725] Code: 48 39 ce 77 9e f3 c3 0f 1f 44 00 00 31 c0 c3 66 66 66 
 66 2e 0f 1f 84 00 00 00 00 00 55 48 89 e5 53 48 89 fb
 48 83 ec 08 48 8b 17 48 8b 8a 90 00 00 00 48 39 4f 40 74 34 80 3d a6 82 
 5b 04 00 75
 [ 1857.283725] RIP  [81229d0f] 
 anon_vma_interval_tree_verify+0xf/0xa0
 [ 1857.283725]  RSP 88007df0fce8
 [ 1857.283725] CR2: 0090
 [ 1858.611277] ---[ end trace b51cc425e9b07fc0 ]---

 The obvious part is that anon_vma_interval_tree_verify() got called with 
 node == NULL, but when
 looking at the caller:

 list_for_each_entry(avc, vma-anon_vma_chain, same_vma)
 anon_vma_interval_tree_verify(avc);

 How it got called with said NULL becomes less obvious.

 I've hit a similar one with today's -next. It isn't exactly the same, but
 I suspect it's the same issue.

 [ 1523.657950] BUG: unable to handle kernel paging request at 
 fff0
 [ 1523.660022] IP: [8122c29c] 
 anon_vma_interval_tree_verify+0xc/0xa0
 [ 1523.660022] PGD 4e28067 PUD 4e29067 PMD 0
 [ 1523.675725] Oops:  [#1] PREEMPT SMP DEBUG_PAGEALLOC
 [ 1523.750066] CPU 0
 [ 1523.750066] Pid: 9050, comm: trinity-child64 Tainted: GW
 3.7.0-rc2-next-20121025-sasha-1-g673f98e-dirty #77
 [ 1523.750066] RIP: 0010:[8122c29c]  [8122c29c] 
 anon_vma_interval_tree_verify+0xc/0xa0
 [ 1523.750066] RSP: 0018:880045f81d48  EFLAGS: 00010296
 [ 1523.750066] RAX:  RBX: fff0 RCX: 
 
 [ 1523.750066] RDX:  RSI: 0001 RDI: 
 fff0
 [ 1523.750066] RBP: 880045f81d58 R08:  R09: 
 0f14
 [ 1523.750066] R10: 0f12 R11:  R12: 
 8800096c8d70
 [ 1523.750066] R13: 8800096c8d00 R14:  R15: 
 8800095b45e0
 [ 1523.750066] FS:  7f7a923f3700() GS:88001360() 
 knlGS:
 [ 1523.750066] CS:  0010 DS:  ES:  CR0: 80050033
 [ 1523.750066] CR2: fff0

Re: mm: NULL ptr deref in anon_vma_interval_tree_verify

2012-11-04 Thread Michel Lespinasse 
On Sun, Nov 4, 2012 at 6:20 PM, Bob Liu lliu...@gmail.com wrote:
 The loop for each entry of vma-anon_vma_chain in validate_mm() is not
 protected by anon_vma lock.
 I think that may be the cause.

 Michel, What's your opinion？

Good catch, I think that's it. Somehow it had not occured to me to
verify the checker code - as in, who's checking the checker ? :)

-- 
Michel Walken Lespinasse
A program is never fully debugged until the last user dies.
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: mm: NULL ptr deref in anon_vma_interval_tree_verify

2012-11-04 Thread Bob Liu 
On Mon, Nov 5, 2012 at 11:31 AM, Michel Lespinasse wal...@google.com wrote:
 On Sun, Nov 4, 2012 at 6:20 PM, Bob Liu lliu...@gmail.com wrote:
 The loop for each entry of vma-anon_vma_chain in validate_mm() is not
 protected by anon_vma lock.
 I think that may be the cause.

 Michel, What's your opinion？

 Good catch, I think that's it. Somehow it had not occured to me to

Hmm, I attached a simple fix patch.
Sasha,
Could you have a test to see whether it can fix your issue?

Thanks,
-Bob


0001-mm-add-anon_vma_lock-to-validate_mm.patch
Description: Binary data

Re: mm: NULL ptr deref in anon_vma_interval_tree_verify

2012-11-04 Thread Michel Lespinasse 
On Sun, Nov 4, 2012 at 8:14 PM, Bob Liu lliu...@gmail.com wrote:
 Hmm, I attached a simple fix patch.

Reviewed-by: Michel Lespinasse wal...@google.com
(also ran some tests with it, but I could never reproduce the original
issue anyway).

Bob, it would be easier if you had sent the original patch inline
rather than as an attachment :)

Andrew, can you get this simple fix into your -mm tree ?

-- 
Michel Walken Lespinasse
A program is never fully debugged until the last user dies.
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: mm: NULL ptr deref in anon_vma_interval_tree_verify

2012-11-02 Thread Sasha Levin 
Ping?

On Thu, Oct 25, 2012 at 4:26 PM, Sasha Levin  wrote:
> On 10/18/2012 06:46 PM, Sasha Levin wrote:
>> Hi all,
>>
>> While fuzzing with trinity inside a KVM tools (lkvm) guest, on today's 
>> linux-next kernel,
>> I saw the following:
>>
>> [ 1857.278176] BUG: unable to handle kernel NULL pointer dereference at 
>> 0090
>> [ 1857.283725] IP: [] 
>> anon_vma_interval_tree_verify+0xf/0xa0
>> [ 1857.283725] PGD 6e19e067 PUD 6e19f067 PMD 0
>> [ 1857.283725] Oops:  [#1] PREEMPT SMP DEBUG_PAGEALLOC
>> [ 1857.283725] Dumping ftrace buffer:
>> [ 1857.283725](ftrace buffer empty)
>> [ 1857.283725] CPU 2
>> [ 1857.283725] Pid: 15637, comm: trinity-child18 Tainted: GW
>> 3.7.0-rc1-next-20121018-sasha-2-g60a870d-dirty #61
>> [ 1857.283725] RIP: 0010:[]  [] 
>> anon_vma_interval_tree_verify+0xf/0xa0
>> [ 1857.283725] RSP: 0018:88007df0fce8  EFLAGS: 00010296
>> [ 1857.283725] RAX: 880089db1000 RBX: 880089db0ff0 RCX: 
>> 8800869e6928
>> [ 1857.283725] RDX:  RSI: 880089db1008 RDI: 
>> 880089db0ff0
>> [ 1857.283725] RBP: 88007df0fcf8 R08: 88006427d508 R09: 
>> 88012bb95f20
>> [ 1857.283725] R10: 0001 R11: 8800c8525c60 R12: 
>> 88006e199370
>> [ 1857.283725] R13: 88006e199300 R14:  R15: 
>> 880089db1000
>> [ 1857.283725] FS:  7f322fd4c700() GS:88004d60() 
>> knlGS:
>> [ 1857.283725] CS:  0010 DS:  ES:  CR0: 80050033
>> [ 1857.283725] CR2: 0090 CR3: 6e19d000 CR4: 
>> 000406e0
>> [ 1857.283725] DR0:  DR1:  DR2: 
>> 
>> [ 1857.283725] DR3:  DR6: 0ff0 DR7: 
>> 0400
>> [ 1857.283725] Process trinity-child18 (pid: 15637, threadinfo 
>> 88007df0e000, task 88007ac8)
>> [ 1857.283725] Stack:
>> [ 1857.283725]  88007df0fd38 880089db0ff0 88007df0fd48 
>> 81233b58
>> [ 1857.283725]  88007df0fd38 880089db1000 80d0 
>> 880089db1000
>> [ 1857.283725]  88012bb95f20 885d97c8 885d97d8 
>> 880089db1000
>> [ 1857.283725] Call Trace:
>> [ 1857.283725]  [] validate_mm+0x58/0x1e0
>> [ 1857.283725]  [] vma_link+0x94/0xe0
>> [ 1857.283725]  [] ? _raw_spin_unlock_irqrestore+0x84/0xb0
>> [ 1857.283725]  [] mmap_region+0x3f5/0x5c0
>> [ 1857.283725]  [] do_mmap_pgoff+0x2b7/0x330
>> [ 1857.283725]  [] ? vm_mmap_pgoff+0x61/0xa0
>> [ 1857.283725]  [] vm_mmap_pgoff+0x7a/0xa0
>> [ 1857.283725]  [] sys_mmap_pgoff+0x182/0x1a0
>> [ 1857.283725]  [] ? syscall_trace_enter+0x20/0x2e0
>> [ 1857.283725]  [] sys_mmap+0x1d/0x20
>> [ 1857.283725]  [] tracesys+0xe1/0xe6
>> [ 1857.283725] Code: 48 39 ce 77 9e f3 c3 0f 1f 44 00 00 31 c0 c3 66 66 66 
>> 66 2e 0f 1f 84 00 00 00 00 00 55 48 89 e5 53 48 89 fb
>> 48 83 ec 08 48 8b 17 <48> 8b 8a 90 00 00 00 48 39 4f 40 74 34 80 3d a6 82 5b 
>> 04 00 75
>> [ 1857.283725] RIP  [] 
>> anon_vma_interval_tree_verify+0xf/0xa0
>> [ 1857.283725]  RSP 
>> [ 1857.283725] CR2: 0090
>> [ 1858.611277] ---[ end trace b51cc425e9b07fc0 ]---
>>
>> The obvious part is that anon_vma_interval_tree_verify() got called with 
>> node == NULL, but when
>> looking at the caller:
>>
>> list_for_each_entry(avc, >anon_vma_chain, same_vma)
>> anon_vma_interval_tree_verify(avc);
>>
>> How it got called with said NULL becomes less obvious.
>
> I've hit a similar one with today's -next. It isn't exactly the same, but
> I suspect it's the same issue.
>
> [ 1523.657950] BUG: unable to handle kernel paging request at fff0
> [ 1523.660022] IP: [] anon_vma_interval_tree_verify+0xc/0xa0
> [ 1523.660022] PGD 4e28067 PUD 4e29067 PMD 0
> [ 1523.675725] Oops:  [#1] PREEMPT SMP DEBUG_PAGEALLOC
> [ 1523.750066] CPU 0
> [ 1523.750066] Pid: 9050, comm: trinity-child64 Tainted: GW
> 3.7.0-rc2-next-20121025-sasha-1-g673f98e-dirty #77
> [ 1523.750066] RIP: 0010:[]  [] 
> anon_vma_interval_tree_verify+0xc/0xa0
> [ 1523.750066] RSP: 0018:880045f81d48  EFLAGS: 00010296
> [ 1523.750066] RAX:  RBX: fff0 RCX: 
> 
> [ 1523.750066] RDX:  RSI: 0001 RDI: 
> fff0
> [ 1523.750066] RBP: 880045f81d58 R08:  R09: 
> 0f14
> [ 1523.750066] R10: 0f12 R11:  R12: 
> 8800096c8d70
> [ 1523.750066] R13: 8800096c8d00 R14:  R15: 
> 8800095b45e0
> [ 1523.750066] FS:  7f7a923f3700() GS:88001360() 
> knlGS:
> [ 1523.750066] CS:  0010 DS:  ES:  CR0: 80050033
> [ 1523.750066] CR2: fff0 CR3: 0969d000 CR4: 
> 000406f0
> [ 1523.750066] DR0:  DR1:  DR2: 
> 
> [ 1523.750066] DR3:  DR6: 0ff0 DR7: 
> 0400
> [

Re: mm: NULL ptr deref in anon_vma_interval_tree_verify

2012-11-02 Thread Sasha Levin 
Ping?

On Thu, Oct 25, 2012 at 4:26 PM, Sasha Levin levinsasha...@gmail.com wrote:
 On 10/18/2012 06:46 PM, Sasha Levin wrote:
 Hi all,

 While fuzzing with trinity inside a KVM tools (lkvm) guest, on today's 
 linux-next kernel,
 I saw the following:

 [ 1857.278176] BUG: unable to handle kernel NULL pointer dereference at 
 0090
 [ 1857.283725] IP: [81229d0f] 
 anon_vma_interval_tree_verify+0xf/0xa0
 [ 1857.283725] PGD 6e19e067 PUD 6e19f067 PMD 0
 [ 1857.283725] Oops:  [#1] PREEMPT SMP DEBUG_PAGEALLOC
 [ 1857.283725] Dumping ftrace buffer:
 [ 1857.283725](ftrace buffer empty)
 [ 1857.283725] CPU 2
 [ 1857.283725] Pid: 15637, comm: trinity-child18 Tainted: GW
 3.7.0-rc1-next-20121018-sasha-2-g60a870d-dirty #61
 [ 1857.283725] RIP: 0010:[81229d0f]  [81229d0f] 
 anon_vma_interval_tree_verify+0xf/0xa0
 [ 1857.283725] RSP: 0018:88007df0fce8  EFLAGS: 00010296
 [ 1857.283725] RAX: 880089db1000 RBX: 880089db0ff0 RCX: 
 8800869e6928
 [ 1857.283725] RDX:  RSI: 880089db1008 RDI: 
 880089db0ff0
 [ 1857.283725] RBP: 88007df0fcf8 R08: 88006427d508 R09: 
 88012bb95f20
 [ 1857.283725] R10: 0001 R11: 8800c8525c60 R12: 
 88006e199370
 [ 1857.283725] R13: 88006e199300 R14:  R15: 
 880089db1000
 [ 1857.283725] FS:  7f322fd4c700() GS:88004d60() 
 knlGS:
 [ 1857.283725] CS:  0010 DS:  ES:  CR0: 80050033
 [ 1857.283725] CR2: 0090 CR3: 6e19d000 CR4: 
 000406e0
 [ 1857.283725] DR0:  DR1:  DR2: 
 
 [ 1857.283725] DR3:  DR6: 0ff0 DR7: 
 0400
 [ 1857.283725] Process trinity-child18 (pid: 15637, threadinfo 
 88007df0e000, task 88007ac8)
 [ 1857.283725] Stack:
 [ 1857.283725]  88007df0fd38 880089db0ff0 88007df0fd48 
 81233b58
 [ 1857.283725]  88007df0fd38 880089db1000 80d0 
 880089db1000
 [ 1857.283725]  88012bb95f20 885d97c8 885d97d8 
 880089db1000
 [ 1857.283725] Call Trace:
 [ 1857.283725]  [81233b58] validate_mm+0x58/0x1e0
 [ 1857.283725]  [81233da4] vma_link+0x94/0xe0
 [ 1857.283725]  [83a67fd4] ? _raw_spin_unlock_irqrestore+0x84/0xb0
 [ 1857.283725]  [81235f75] mmap_region+0x3f5/0x5c0
 [ 1857.283725]  [812363f7] do_mmap_pgoff+0x2b7/0x330
 [ 1857.283725]  [81220fd1] ? vm_mmap_pgoff+0x61/0xa0
 [ 1857.283725]  [81220fea] vm_mmap_pgoff+0x7a/0xa0
 [ 1857.283725]  [81234c72] sys_mmap_pgoff+0x182/0x1a0
 [ 1857.283725]  [8107dc40] ? syscall_trace_enter+0x20/0x2e0
 [ 1857.283725]  [810738dd] sys_mmap+0x1d/0x20
 [ 1857.283725]  [83a69ad8] tracesys+0xe1/0xe6
 [ 1857.283725] Code: 48 39 ce 77 9e f3 c3 0f 1f 44 00 00 31 c0 c3 66 66 66 
 66 2e 0f 1f 84 00 00 00 00 00 55 48 89 e5 53 48 89 fb
 48 83 ec 08 48 8b 17 48 8b 8a 90 00 00 00 48 39 4f 40 74 34 80 3d a6 82 5b 
 04 00 75
 [ 1857.283725] RIP  [81229d0f] 
 anon_vma_interval_tree_verify+0xf/0xa0
 [ 1857.283725]  RSP 88007df0fce8
 [ 1857.283725] CR2: 0090
 [ 1858.611277] ---[ end trace b51cc425e9b07fc0 ]---

 The obvious part is that anon_vma_interval_tree_verify() got called with 
 node == NULL, but when
 looking at the caller:

 list_for_each_entry(avc, vma-anon_vma_chain, same_vma)
 anon_vma_interval_tree_verify(avc);

 How it got called with said NULL becomes less obvious.

 I've hit a similar one with today's -next. It isn't exactly the same, but
 I suspect it's the same issue.

 [ 1523.657950] BUG: unable to handle kernel paging request at fff0
 [ 1523.660022] IP: [8122c29c] anon_vma_interval_tree_verify+0xc/0xa0
 [ 1523.660022] PGD 4e28067 PUD 4e29067 PMD 0
 [ 1523.675725] Oops:  [#1] PREEMPT SMP DEBUG_PAGEALLOC
 [ 1523.750066] CPU 0
 [ 1523.750066] Pid: 9050, comm: trinity-child64 Tainted: GW
 3.7.0-rc2-next-20121025-sasha-1-g673f98e-dirty #77
 [ 1523.750066] RIP: 0010:[8122c29c]  [8122c29c] 
 anon_vma_interval_tree_verify+0xc/0xa0
 [ 1523.750066] RSP: 0018:880045f81d48  EFLAGS: 00010296
 [ 1523.750066] RAX:  RBX: fff0 RCX: 
 
 [ 1523.750066] RDX:  RSI: 0001 RDI: 
 fff0
 [ 1523.750066] RBP: 880045f81d58 R08:  R09: 
 0f14
 [ 1523.750066] R10: 0f12 R11:  R12: 
 8800096c8d70
 [ 1523.750066] R13: 8800096c8d00 R14:  R15: 
 8800095b45e0
 [ 1523.750066] FS:  7f7a923f3700() GS:88001360() 
 knlGS:
 [ 1523.750066] CS:  0010 DS:  ES:  CR0: 80050033
 [ 1523.750066] CR2: fff0 CR3: 0969d000 CR4: 
 000406f0
 [ 1523.750066] DR0:

Re: mm: NULL ptr deref in anon_vma_interval_tree_verify

2012-10-25 Thread Sasha Levin 
On 10/18/2012 06:46 PM, Sasha Levin wrote:
> Hi all,
> 
> While fuzzing with trinity inside a KVM tools (lkvm) guest, on today's 
> linux-next kernel,
> I saw the following:
> 
> [ 1857.278176] BUG: unable to handle kernel NULL pointer dereference at 
> 0090
> [ 1857.283725] IP: [] anon_vma_interval_tree_verify+0xf/0xa0
> [ 1857.283725] PGD 6e19e067 PUD 6e19f067 PMD 0
> [ 1857.283725] Oops:  [#1] PREEMPT SMP DEBUG_PAGEALLOC
> [ 1857.283725] Dumping ftrace buffer:
> [ 1857.283725](ftrace buffer empty)
> [ 1857.283725] CPU 2
> [ 1857.283725] Pid: 15637, comm: trinity-child18 Tainted: GW
> 3.7.0-rc1-next-20121018-sasha-2-g60a870d-dirty #61
> [ 1857.283725] RIP: 0010:[]  [] 
> anon_vma_interval_tree_verify+0xf/0xa0
> [ 1857.283725] RSP: 0018:88007df0fce8  EFLAGS: 00010296
> [ 1857.283725] RAX: 880089db1000 RBX: 880089db0ff0 RCX: 
> 8800869e6928
> [ 1857.283725] RDX:  RSI: 880089db1008 RDI: 
> 880089db0ff0
> [ 1857.283725] RBP: 88007df0fcf8 R08: 88006427d508 R09: 
> 88012bb95f20
> [ 1857.283725] R10: 0001 R11: 8800c8525c60 R12: 
> 88006e199370
> [ 1857.283725] R13: 88006e199300 R14:  R15: 
> 880089db1000
> [ 1857.283725] FS:  7f322fd4c700() GS:88004d60() 
> knlGS:
> [ 1857.283725] CS:  0010 DS:  ES:  CR0: 80050033
> [ 1857.283725] CR2: 0090 CR3: 6e19d000 CR4: 
> 000406e0
> [ 1857.283725] DR0:  DR1:  DR2: 
> 
> [ 1857.283725] DR3:  DR6: 0ff0 DR7: 
> 0400
> [ 1857.283725] Process trinity-child18 (pid: 15637, threadinfo 
> 88007df0e000, task 88007ac8)
> [ 1857.283725] Stack:
> [ 1857.283725]  88007df0fd38 880089db0ff0 88007df0fd48 
> 81233b58
> [ 1857.283725]  88007df0fd38 880089db1000 80d0 
> 880089db1000
> [ 1857.283725]  88012bb95f20 885d97c8 885d97d8 
> 880089db1000
> [ 1857.283725] Call Trace:
> [ 1857.283725]  [] validate_mm+0x58/0x1e0
> [ 1857.283725]  [] vma_link+0x94/0xe0
> [ 1857.283725]  [] ? _raw_spin_unlock_irqrestore+0x84/0xb0
> [ 1857.283725]  [] mmap_region+0x3f5/0x5c0
> [ 1857.283725]  [] do_mmap_pgoff+0x2b7/0x330
> [ 1857.283725]  [] ? vm_mmap_pgoff+0x61/0xa0
> [ 1857.283725]  [] vm_mmap_pgoff+0x7a/0xa0
> [ 1857.283725]  [] sys_mmap_pgoff+0x182/0x1a0
> [ 1857.283725]  [] ? syscall_trace_enter+0x20/0x2e0
> [ 1857.283725]  [] sys_mmap+0x1d/0x20
> [ 1857.283725]  [] tracesys+0xe1/0xe6
> [ 1857.283725] Code: 48 39 ce 77 9e f3 c3 0f 1f 44 00 00 31 c0 c3 66 66 66 66 
> 2e 0f 1f 84 00 00 00 00 00 55 48 89 e5 53 48 89 fb
> 48 83 ec 08 48 8b 17 <48> 8b 8a 90 00 00 00 48 39 4f 40 74 34 80 3d a6 82 5b 
> 04 00 75
> [ 1857.283725] RIP  [] 
> anon_vma_interval_tree_verify+0xf/0xa0
> [ 1857.283725]  RSP 
> [ 1857.283725] CR2: 0090
> [ 1858.611277] ---[ end trace b51cc425e9b07fc0 ]---
> 
> The obvious part is that anon_vma_interval_tree_verify() got called with node 
> == NULL, but when
> looking at the caller:
> 
> list_for_each_entry(avc, >anon_vma_chain, same_vma)
> anon_vma_interval_tree_verify(avc);
> 
> How it got called with said NULL becomes less obvious.

I've hit a similar one with today's -next. It isn't exactly the same, but
I suspect it's the same issue.

[ 1523.657950] BUG: unable to handle kernel paging request at fff0
[ 1523.660022] IP: [] anon_vma_interval_tree_verify+0xc/0xa0
[ 1523.660022] PGD 4e28067 PUD 4e29067 PMD 0
[ 1523.675725] Oops:  [#1] PREEMPT SMP DEBUG_PAGEALLOC
[ 1523.750066] CPU 0
[ 1523.750066] Pid: 9050, comm: trinity-child64 Tainted: GW
3.7.0-rc2-next-20121025-sasha-1-g673f98e-dirty #77
[ 1523.750066] RIP: 0010:[]  [] 
anon_vma_interval_tree_verify+0xc/0xa0
[ 1523.750066] RSP: 0018:880045f81d48  EFLAGS: 00010296
[ 1523.750066] RAX:  RBX: fff0 RCX: 
[ 1523.750066] RDX:  RSI: 0001 RDI: fff0
[ 1523.750066] RBP: 880045f81d58 R08:  R09: 0f14
[ 1523.750066] R10: 0f12 R11:  R12: 8800096c8d70
[ 1523.750066] R13: 8800096c8d00 R14:  R15: 8800095b45e0
[ 1523.750066] FS:  7f7a923f3700() GS:88001360() 
knlGS:
[ 1523.750066] CS:  0010 DS:  ES:  CR0: 80050033
[ 1523.750066] CR2: fff0 CR3: 0969d000 CR4: 000406f0
[ 1523.750066] DR0:  DR1:  DR2: 
[ 1523.750066] DR3:  DR6: 0ff0 DR7: 0400
[ 1523.750066] Process trinity-child64 (pid: 9050, threadinfo 880045f8, 
task 880048eb)
[ 1523.750066] Stack:
[ 1523.750066]  88000d7533f0 fff0 880045f81da8 
812361d8
[

Re: mm: NULL ptr deref in anon_vma_interval_tree_verify

2012-10-25 Thread Sasha Levin 
On 10/18/2012 06:46 PM, Sasha Levin wrote:
 Hi all,
 
 While fuzzing with trinity inside a KVM tools (lkvm) guest, on today's 
 linux-next kernel,
 I saw the following:
 
 [ 1857.278176] BUG: unable to handle kernel NULL pointer dereference at 
 0090
 [ 1857.283725] IP: [81229d0f] anon_vma_interval_tree_verify+0xf/0xa0
 [ 1857.283725] PGD 6e19e067 PUD 6e19f067 PMD 0
 [ 1857.283725] Oops:  [#1] PREEMPT SMP DEBUG_PAGEALLOC
 [ 1857.283725] Dumping ftrace buffer:
 [ 1857.283725](ftrace buffer empty)
 [ 1857.283725] CPU 2
 [ 1857.283725] Pid: 15637, comm: trinity-child18 Tainted: GW
 3.7.0-rc1-next-20121018-sasha-2-g60a870d-dirty #61
 [ 1857.283725] RIP: 0010:[81229d0f]  [81229d0f] 
 anon_vma_interval_tree_verify+0xf/0xa0
 [ 1857.283725] RSP: 0018:88007df0fce8  EFLAGS: 00010296
 [ 1857.283725] RAX: 880089db1000 RBX: 880089db0ff0 RCX: 
 8800869e6928
 [ 1857.283725] RDX:  RSI: 880089db1008 RDI: 
 880089db0ff0
 [ 1857.283725] RBP: 88007df0fcf8 R08: 88006427d508 R09: 
 88012bb95f20
 [ 1857.283725] R10: 0001 R11: 8800c8525c60 R12: 
 88006e199370
 [ 1857.283725] R13: 88006e199300 R14:  R15: 
 880089db1000
 [ 1857.283725] FS:  7f322fd4c700() GS:88004d60() 
 knlGS:
 [ 1857.283725] CS:  0010 DS:  ES:  CR0: 80050033
 [ 1857.283725] CR2: 0090 CR3: 6e19d000 CR4: 
 000406e0
 [ 1857.283725] DR0:  DR1:  DR2: 
 
 [ 1857.283725] DR3:  DR6: 0ff0 DR7: 
 0400
 [ 1857.283725] Process trinity-child18 (pid: 15637, threadinfo 
 88007df0e000, task 88007ac8)
 [ 1857.283725] Stack:
 [ 1857.283725]  88007df0fd38 880089db0ff0 88007df0fd48 
 81233b58
 [ 1857.283725]  88007df0fd38 880089db1000 80d0 
 880089db1000
 [ 1857.283725]  88012bb95f20 885d97c8 885d97d8 
 880089db1000
 [ 1857.283725] Call Trace:
 [ 1857.283725]  [81233b58] validate_mm+0x58/0x1e0
 [ 1857.283725]  [81233da4] vma_link+0x94/0xe0
 [ 1857.283725]  [83a67fd4] ? _raw_spin_unlock_irqrestore+0x84/0xb0
 [ 1857.283725]  [81235f75] mmap_region+0x3f5/0x5c0
 [ 1857.283725]  [812363f7] do_mmap_pgoff+0x2b7/0x330
 [ 1857.283725]  [81220fd1] ? vm_mmap_pgoff+0x61/0xa0
 [ 1857.283725]  [81220fea] vm_mmap_pgoff+0x7a/0xa0
 [ 1857.283725]  [81234c72] sys_mmap_pgoff+0x182/0x1a0
 [ 1857.283725]  [8107dc40] ? syscall_trace_enter+0x20/0x2e0
 [ 1857.283725]  [810738dd] sys_mmap+0x1d/0x20
 [ 1857.283725]  [83a69ad8] tracesys+0xe1/0xe6
 [ 1857.283725] Code: 48 39 ce 77 9e f3 c3 0f 1f 44 00 00 31 c0 c3 66 66 66 66 
 2e 0f 1f 84 00 00 00 00 00 55 48 89 e5 53 48 89 fb
 48 83 ec 08 48 8b 17 48 8b 8a 90 00 00 00 48 39 4f 40 74 34 80 3d a6 82 5b 
 04 00 75
 [ 1857.283725] RIP  [81229d0f] 
 anon_vma_interval_tree_verify+0xf/0xa0
 [ 1857.283725]  RSP 88007df0fce8
 [ 1857.283725] CR2: 0090
 [ 1858.611277] ---[ end trace b51cc425e9b07fc0 ]---
 
 The obvious part is that anon_vma_interval_tree_verify() got called with node 
 == NULL, but when
 looking at the caller:
 
 list_for_each_entry(avc, vma-anon_vma_chain, same_vma)
 anon_vma_interval_tree_verify(avc);
 
 How it got called with said NULL becomes less obvious.

I've hit a similar one with today's -next. It isn't exactly the same, but
I suspect it's the same issue.

[ 1523.657950] BUG: unable to handle kernel paging request at fff0
[ 1523.660022] IP: [8122c29c] anon_vma_interval_tree_verify+0xc/0xa0
[ 1523.660022] PGD 4e28067 PUD 4e29067 PMD 0
[ 1523.675725] Oops:  [#1] PREEMPT SMP DEBUG_PAGEALLOC
[ 1523.750066] CPU 0
[ 1523.750066] Pid: 9050, comm: trinity-child64 Tainted: GW
3.7.0-rc2-next-20121025-sasha-1-g673f98e-dirty #77
[ 1523.750066] RIP: 0010:[8122c29c]  [8122c29c] 
anon_vma_interval_tree_verify+0xc/0xa0
[ 1523.750066] RSP: 0018:880045f81d48  EFLAGS: 00010296
[ 1523.750066] RAX:  RBX: fff0 RCX: 
[ 1523.750066] RDX:  RSI: 0001 RDI: fff0
[ 1523.750066] RBP: 880045f81d58 R08:  R09: 0f14
[ 1523.750066] R10: 0f12 R11:  R12: 8800096c8d70
[ 1523.750066] R13: 8800096c8d00 R14:  R15: 8800095b45e0
[ 1523.750066] FS:  7f7a923f3700() GS:88001360() 
knlGS:
[ 1523.750066] CS:  0010 DS:  ES:  CR0: 80050033
[ 1523.750066] CR2: fff0 CR3: 0969d000 CR4: 000406f0
[ 1523.750066] DR0:  DR1:  DR2: 
[ 1523.750066] DR3:  DR6: 0ff0 DR7: