* [PATCH] Uprobes: Fix kernel oops with delayed_uprobe_remove()
@ 2018-11-14 8:19 Ravi Bangoria
2018-11-14 16:06 ` Oleg Nesterov
0 siblings, 1 reply; 9+ messages in thread
From: Ravi Bangoria @ 2018-11-14 8:19 UTC (permalink / raw)
To: srikar, oleg, rostedt, mhiramat, liu.song.a23
Cc: peterz, mingo, acme, alexander.shishkin, jolsa, namhyung,
linux-kernel, ananth, alexis.berlemont, naveen.n.rao,
linux-arm-kernel, linux-mips, linux, ralf, paul.burton,
Ravi Bangoria
syzbot reported a kernel crash with delayed_uprobe_remove():
https://lkml.org/lkml/2018/11/1/1244
Backtrace mentioned in the link points to a race between process
exit and uprobe_unregister(). Fix it by locking delayed_uprobe_lock
before calling delayed_uprobe_remove() from put_uprobe().
Reported-by: syzbot+cb1fb754b771caca0a88@syzkaller.appspotmail.com
Signed-off-by: Ravi Bangoria <ravi.bangoria@linux.ibm.com>
---
kernel/events/uprobes.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/kernel/events/uprobes.c b/kernel/events/uprobes.c
index 96fb51f3994f..e527c4753d4f 100644
--- a/kernel/events/uprobes.c
+++ b/kernel/events/uprobes.c
@@ -572,7 +572,9 @@ static void put_uprobe(struct uprobe *uprobe)
* gets called, we don't get a chance to remove uprobe from
* delayed_uprobe_list from remove_breakpoint(). Do it here.
*/
+ mutex_lock(&delayed_uprobe_lock);
delayed_uprobe_remove(uprobe, NULL);
+ mutex_unlock(&delayed_uprobe_lock);
kfree(uprobe);
}
}
--
2.19.1
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH] Uprobes: Fix kernel oops with delayed_uprobe_remove()
2018-11-14 8:19 [PATCH] Uprobes: Fix kernel oops with delayed_uprobe_remove() Ravi Bangoria
@ 2018-11-14 16:06 ` Oleg Nesterov
2018-11-15 4:06 ` Ravi Bangoria
0 siblings, 1 reply; 9+ messages in thread
From: Oleg Nesterov @ 2018-11-14 16:06 UTC (permalink / raw)
To: Ravi Bangoria
Cc: srikar, rostedt, mhiramat, liu.song.a23, peterz, mingo, acme,
alexander.shishkin, jolsa, namhyung, linux-kernel, ananth,
alexis.berlemont, naveen.n.rao, linux-arm-kernel, linux-mips,
linux, ralf, paul.burton
On 11/14, Ravi Bangoria wrote:
>
> syzbot reported a kernel crash with delayed_uprobe_remove():
> https://lkml.org/lkml/2018/11/1/1244
>
> Backtrace mentioned in the link points to a race between process
> exit and uprobe_unregister(). Fix it by locking delayed_uprobe_lock
> before calling delayed_uprobe_remove() from put_uprobe().
The patch looks good to me, but could you update the changelog?
Please explain that the exiting task calls uprobe_clear_state() which
can race with delayed_uprobe_remove(). IIUC this is the only problem
solved by this patch, right?
Oleg.
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] Uprobes: Fix kernel oops with delayed_uprobe_remove()
2018-11-14 16:06 ` Oleg Nesterov
@ 2018-11-15 4:06 ` Ravi Bangoria
2018-11-15 12:43 ` Oleg Nesterov
0 siblings, 1 reply; 9+ messages in thread
From: Ravi Bangoria @ 2018-11-15 4:06 UTC (permalink / raw)
To: Oleg Nesterov
Cc: srikar, rostedt, mhiramat, liu.song.a23, peterz, mingo, acme,
alexander.shishkin, jolsa, namhyung, linux-kernel, ananth,
alexis.berlemont, naveen.n.rao, linux-arm-kernel, linux-mips,
linux, ralf, paul.burton, Ravi Bangoria
Hi Oleg,
On 11/14/18 9:36 PM, Oleg Nesterov wrote:
> On 11/14, Ravi Bangoria wrote:
>>
>> syzbot reported a kernel crash with delayed_uprobe_remove():
>> https://lkml.org/lkml/2018/11/1/1244
>>
>> Backtrace mentioned in the link points to a race between process
>> exit and uprobe_unregister(). Fix it by locking delayed_uprobe_lock
>> before calling delayed_uprobe_remove() from put_uprobe().
>
> The patch looks good to me, but could you update the changelog?
>
> Please explain that the exiting task calls uprobe_clear_state() which
> can race with delayed_uprobe_remove(). IIUC this is the only problem
> solved by this patch, right?
Right. Is this better:
There could be a race between task exit and probe unregister:
exit_mm()
mmput()
__mmput() uprobe_unregister()
uprobe_clear_state() put_uprobe()
delayed_uprobe_remove() delayed_uprobe_remove()
put_uprobe() is calling delayed_uprobe_remove() without taking
delayed_uprobe_lock and thus the race sometimes results in a
kernel crash. Fix this by taking delayed_uprobe_lock before
calling delayed_uprobe_remove() from put_uprobe().
Detailed crash log can be found at:
https://lkml.org/lkml/2018/11/1/1244
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] Uprobes: Fix kernel oops with delayed_uprobe_remove()
2018-11-15 4:06 ` Ravi Bangoria
@ 2018-11-15 12:43 ` Oleg Nesterov
2018-12-03 6:22 ` Ravi Bangoria
0 siblings, 1 reply; 9+ messages in thread
From: Oleg Nesterov @ 2018-11-15 12:43 UTC (permalink / raw)
To: Ravi Bangoria
Cc: srikar, rostedt, mhiramat, liu.song.a23, peterz, mingo, acme,
alexander.shishkin, jolsa, namhyung, linux-kernel, ananth,
alexis.berlemont, naveen.n.rao, linux-arm-kernel, linux-mips,
linux, ralf, paul.burton
On 11/15, Ravi Bangoria wrote:
>
> There could be a race between task exit and probe unregister:
>
> exit_mm()
> mmput()
> __mmput() uprobe_unregister()
> uprobe_clear_state() put_uprobe()
> delayed_uprobe_remove() delayed_uprobe_remove()
>
> put_uprobe() is calling delayed_uprobe_remove() without taking
> delayed_uprobe_lock and thus the race sometimes results in a
> kernel crash. Fix this by taking delayed_uprobe_lock before
> calling delayed_uprobe_remove() from put_uprobe().
>
> Detailed crash log can be found at:
> https://lkml.org/lkml/2018/11/1/1244
Thanks, looks good,
Oleg.
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] Uprobes: Fix kernel oops with delayed_uprobe_remove()
2018-11-15 12:43 ` Oleg Nesterov
@ 2018-12-03 6:22 ` Ravi Bangoria
2018-12-04 5:58 ` Steven Rostedt
0 siblings, 1 reply; 9+ messages in thread
From: Ravi Bangoria @ 2018-12-03 6:22 UTC (permalink / raw)
To: rostedt
Cc: Oleg Nesterov, srikar, mhiramat, liu.song.a23, peterz, mingo,
acme, alexander.shishkin, jolsa, namhyung, linux-kernel, ananth,
alexis.berlemont, naveen.n.rao, linux-arm-kernel, linux-mips,
linux, ralf, paul.burton
Hi Steve,
Please pull this patch.
Thanks.
On 11/15/18 6:13 PM, Oleg Nesterov wrote:
> On 11/15, Ravi Bangoria wrote:
>>
>> There could be a race between task exit and probe unregister:
>>
>> exit_mm()
>> mmput()
>> __mmput() uprobe_unregister()
>> uprobe_clear_state() put_uprobe()
>> delayed_uprobe_remove() delayed_uprobe_remove()
>>
>> put_uprobe() is calling delayed_uprobe_remove() without taking
>> delayed_uprobe_lock and thus the race sometimes results in a
>> kernel crash. Fix this by taking delayed_uprobe_lock before
>> calling delayed_uprobe_remove() from put_uprobe().
>>
>> Detailed crash log can be found at:
>> https://lkml.org/lkml/2018/11/1/1244
>
> Thanks, looks good,
>
> Oleg.
>
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] Uprobes: Fix kernel oops with delayed_uprobe_remove()
2018-12-03 6:22 ` Ravi Bangoria
@ 2018-12-04 5:58 ` Steven Rostedt
2018-12-05 3:34 ` [PATCH v2] " Ravi Bangoria
0 siblings, 1 reply; 9+ messages in thread
From: Steven Rostedt @ 2018-12-04 5:58 UTC (permalink / raw)
To: Ravi Bangoria
Cc: Oleg Nesterov, srikar, mhiramat, liu.song.a23, peterz, mingo,
acme, alexander.shishkin, jolsa, namhyung, linux-kernel, ananth,
alexis.berlemont, naveen.n.rao, linux-arm-kernel, linux-mips,
linux, ralf, paul.burton
On Mon, 3 Dec 2018 11:52:41 +0530
Ravi Bangoria <ravi.bangoria@linux.ibm.com> wrote:
> Hi Steve,
>
> Please pull this patch.
>
Please send a v2 version of the patch with the updated change log. And
should it have a Fixes and be tagged for stable?
-- Steve
> Thanks.
>
> On 11/15/18 6:13 PM, Oleg Nesterov wrote:
> > On 11/15, Ravi Bangoria wrote:
> >>
> >> There could be a race between task exit and probe unregister:
> >>
> >> exit_mm()
> >> mmput()
> >> __mmput() uprobe_unregister()
> >> uprobe_clear_state() put_uprobe()
> >> delayed_uprobe_remove() delayed_uprobe_remove()
> >>
> >> put_uprobe() is calling delayed_uprobe_remove() without taking
> >> delayed_uprobe_lock and thus the race sometimes results in a
> >> kernel crash. Fix this by taking delayed_uprobe_lock before
> >> calling delayed_uprobe_remove() from put_uprobe().
> >>
> >> Detailed crash log can be found at:
> >> https://lkml.org/lkml/2018/11/1/1244
> >
> > Thanks, looks good,
> >
> > Oleg.
> >
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH v2] Uprobes: Fix kernel oops with delayed_uprobe_remove()
2018-12-04 5:58 ` Steven Rostedt
@ 2018-12-05 3:34 ` Ravi Bangoria
2018-12-05 10:01 ` Srikar Dronamraju
2018-12-05 14:39 ` Oleg Nesterov
0 siblings, 2 replies; 9+ messages in thread
From: Ravi Bangoria @ 2018-12-05 3:34 UTC (permalink / raw)
To: srikar, oleg, rostedt, mhiramat, liu.song.a23
Cc: peterz, mingo, acme, alexander.shishkin, jolsa, namhyung,
linux-kernel, ananth, alexis.berlemont, naveen.n.rao,
linux-arm-kernel, linux-mips, linux, ralf, paul.burton,
Ravi Bangoria
There could be a race between task exit and probe unregister:
exit_mm()
mmput()
__mmput() uprobe_unregister()
uprobe_clear_state() put_uprobe()
delayed_uprobe_remove() delayed_uprobe_remove()
put_uprobe() is calling delayed_uprobe_remove() without taking
delayed_uprobe_lock and thus the race sometimes results in a
kernel crash. Fix this by taking delayed_uprobe_lock before
calling delayed_uprobe_remove() from put_uprobe().
Detailed crash log can be found at:
https://lkml.org/lkml/2018/11/1/1244
Reported-by: syzbot+cb1fb754b771caca0a88@syzkaller.appspotmail.com
Fixes: 1cc33161a83d ("uprobes: Support SDT markers having reference count (semaphore)")
Signed-off-by: Ravi Bangoria <ravi.bangoria@linux.ibm.com>
---
kernel/events/uprobes.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/kernel/events/uprobes.c b/kernel/events/uprobes.c
index 322e97bbb437..abbd8da9ac21 100644
--- a/kernel/events/uprobes.c
+++ b/kernel/events/uprobes.c
@@ -572,7 +572,9 @@ static void put_uprobe(struct uprobe *uprobe)
* gets called, we don't get a chance to remove uprobe from
* delayed_uprobe_list from remove_breakpoint(). Do it here.
*/
+ mutex_lock(&delayed_uprobe_lock);
delayed_uprobe_remove(uprobe, NULL);
+ mutex_unlock(&delayed_uprobe_lock);
kfree(uprobe);
}
}
--
2.19.2
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH v2] Uprobes: Fix kernel oops with delayed_uprobe_remove()
2018-12-05 3:34 ` [PATCH v2] " Ravi Bangoria
@ 2018-12-05 10:01 ` Srikar Dronamraju
2018-12-05 14:39 ` Oleg Nesterov
1 sibling, 0 replies; 9+ messages in thread
From: Srikar Dronamraju @ 2018-12-05 10:01 UTC (permalink / raw)
To: Ravi Bangoria
Cc: oleg, rostedt, mhiramat, liu.song.a23, peterz, mingo, acme,
alexander.shishkin, jolsa, namhyung, linux-kernel, ananth,
alexis.berlemont, naveen.n.rao, linux-arm-kernel, linux-mips,
linux, ralf, paul.burton
* Ravi Bangoria <ravi.bangoria@linux.ibm.com> [2018-12-05 09:04:23]:
> There could be a race between task exit and probe unregister:
>
> exit_mm()
> mmput()
> __mmput() uprobe_unregister()
> uprobe_clear_state() put_uprobe()
> delayed_uprobe_remove() delayed_uprobe_remove()
>
> put_uprobe() is calling delayed_uprobe_remove() without taking
> delayed_uprobe_lock and thus the race sometimes results in a
> kernel crash. Fix this by taking delayed_uprobe_lock before
> calling delayed_uprobe_remove() from put_uprobe().
>
> Detailed crash log can be found at:
> https://lkml.org/lkml/2018/11/1/1244
>
> Reported-by: syzbot+cb1fb754b771caca0a88@syzkaller.appspotmail.com
> Fixes: 1cc33161a83d ("uprobes: Support SDT markers having reference count (semaphore)")
> Signed-off-by: Ravi Bangoria <ravi.bangoria@linux.ibm.com>
Looks good to me.
Reviewed-by: Srikar Dronamraju <srikar@linux.vnet.ibm.com>
> ---
> kernel/events/uprobes.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/kernel/events/uprobes.c b/kernel/events/uprobes.c
> index 322e97bbb437..abbd8da9ac21 100644
> --- a/kernel/events/uprobes.c
> +++ b/kernel/events/uprobes.c
> @@ -572,7 +572,9 @@ static void put_uprobe(struct uprobe *uprobe)
> * gets called, we don't get a chance to remove uprobe from
> * delayed_uprobe_list from remove_breakpoint(). Do it here.
> */
> + mutex_lock(&delayed_uprobe_lock);
> delayed_uprobe_remove(uprobe, NULL);
> + mutex_unlock(&delayed_uprobe_lock);
> kfree(uprobe);
> }
> }
> --
> 2.19.2
>
--
Thanks and Regards
Srikar Dronamraju
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v2] Uprobes: Fix kernel oops with delayed_uprobe_remove()
2018-12-05 3:34 ` [PATCH v2] " Ravi Bangoria
2018-12-05 10:01 ` Srikar Dronamraju
@ 2018-12-05 14:39 ` Oleg Nesterov
1 sibling, 0 replies; 9+ messages in thread
From: Oleg Nesterov @ 2018-12-05 14:39 UTC (permalink / raw)
To: Ravi Bangoria
Cc: srikar, rostedt, mhiramat, liu.song.a23, peterz, mingo, acme,
alexander.shishkin, jolsa, namhyung, linux-kernel, ananth,
alexis.berlemont, naveen.n.rao, linux-arm-kernel, linux-mips,
linux, ralf, paul.burton
On 12/05, Ravi Bangoria wrote:
>
> --- a/kernel/events/uprobes.c
> +++ b/kernel/events/uprobes.c
> @@ -572,7 +572,9 @@ static void put_uprobe(struct uprobe *uprobe)
> * gets called, we don't get a chance to remove uprobe from
> * delayed_uprobe_list from remove_breakpoint(). Do it here.
> */
> + mutex_lock(&delayed_uprobe_lock);
> delayed_uprobe_remove(uprobe, NULL);
> + mutex_unlock(&delayed_uprobe_lock);
> kfree(uprobe);
Acked-by: Oleg Nesterov <oleg@redhat.com>
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2018-12-05 14:39 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-11-14 8:19 [PATCH] Uprobes: Fix kernel oops with delayed_uprobe_remove() Ravi Bangoria
2018-11-14 16:06 ` Oleg Nesterov
2018-11-15 4:06 ` Ravi Bangoria
2018-11-15 12:43 ` Oleg Nesterov
2018-12-03 6:22 ` Ravi Bangoria
2018-12-04 5:58 ` Steven Rostedt
2018-12-05 3:34 ` [PATCH v2] " Ravi Bangoria
2018-12-05 10:01 ` Srikar Dronamraju
2018-12-05 14:39 ` Oleg Nesterov
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).