Re: [PATCH] Uprobes: Fix kernel oops with delayed_uprobe_remove()
On Mon, 3 Dec 2018 11:52:41 +0530 Ravi Bangoria 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. > >
Re: [PATCH] Uprobes: Fix kernel oops with delayed_uprobe_remove()
On Mon, 3 Dec 2018 11:52:41 +0530 Ravi Bangoria 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. > >
Re: [PATCH] Uprobes: Fix kernel oops with delayed_uprobe_remove()
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. >
Re: [PATCH] Uprobes: Fix kernel oops with delayed_uprobe_remove()
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. >
Re: [PATCH] Uprobes: Fix kernel oops with delayed_uprobe_remove()
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.
Re: [PATCH] Uprobes: Fix kernel oops with delayed_uprobe_remove()
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.
Re: [PATCH] Uprobes: Fix kernel oops with delayed_uprobe_remove()
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
Re: [PATCH] Uprobes: Fix kernel oops with delayed_uprobe_remove()
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
Re: [PATCH] Uprobes: Fix kernel oops with delayed_uprobe_remove()
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.
Re: [PATCH] Uprobes: Fix kernel oops with delayed_uprobe_remove()
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.
