* Backporting "padata: Remove broken queue flushing"
@ 2020-05-19 13:53 Ben Hutchings
2020-05-19 20:00 ` Daniel Jordan
0 siblings, 1 reply; 5+ messages in thread
From: Ben Hutchings @ 2020-05-19 13:53 UTC (permalink / raw)
To: Herbert Xu; +Cc: Daniel Jordan, Steffen Klassert, linux-crypto, stable
[-- Attachment #1: Type: text/plain, Size: 424 bytes --]
I noticed that commit 07928d9bfc81 "padata: Remove broken queue
flushing" has been backported to most stable branches, but commit
6fc4dbcf0276 "padata: Replace delayed timer with immediate workqueue in
padata_reorder" has not.
Is this correct? What prevents the parallel_data ref-count from
dropping to 0 while the timer is scheduled?
Ben.
--
Ben Hutchings
Larkinson's Law: All laws are basically false.
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: Backporting "padata: Remove broken queue flushing"
2020-05-19 13:53 Backporting "padata: Remove broken queue flushing" Ben Hutchings
@ 2020-05-19 20:00 ` Daniel Jordan
2020-05-20 14:33 ` Ben Hutchings
0 siblings, 1 reply; 5+ messages in thread
From: Daniel Jordan @ 2020-05-19 20:00 UTC (permalink / raw)
To: Ben Hutchings
Cc: Herbert Xu, Daniel Jordan, Steffen Klassert, linux-crypto, stable
Hello Ben,
On Tue, May 19, 2020 at 02:53:05PM +0100, Ben Hutchings wrote:
> I noticed that commit 07928d9bfc81 "padata: Remove broken queue
> flushing" has been backported to most stable branches, but commit
> 6fc4dbcf0276 "padata: Replace delayed timer with immediate workqueue in
> padata_reorder" has not.
>
> Is this correct? What prevents the parallel_data ref-count from
> dropping to 0 while the timer is scheduled?
Doesn't seem like anything does, looking at 4.19.
I can see a race where the timer function uses a parallel_data after free
whether or not the refcount goes to 0. Don't think it's likely to happen in
practice because of how small the window is between the serial callback
finishing and the timer being deactivated.
task1:
padata_reorder
task2:
padata_do_serial
// object arrives in reorder queue
// sees reorder_objects > 0,
// set timer for 1 second
mod_timer
return
padata_reorder
// queue serial work, which finishes
// (now possibly no more objects
// left)
|
task1: |
// pd is freed one of two ways: |
// 1) pcrypt is unloaded |
// 2) padata_replace triggered |
// from userspace | (small window)
|
task3: |
padata_reorder_timer |
// uses pd after free |
|
del_timer // too late
If I got this right we might want to backport the commit you mentioned to be on
the safe side.
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: Backporting "padata: Remove broken queue flushing"
2020-05-19 20:00 ` Daniel Jordan
@ 2020-05-20 14:33 ` Ben Hutchings
2020-05-21 8:00 ` Greg Kroah-Hartman
0 siblings, 1 reply; 5+ messages in thread
From: Ben Hutchings @ 2020-05-20 14:33 UTC (permalink / raw)
To: Daniel Jordan, Greg Kroah-Hartman, Sasha Levin
Cc: Herbert Xu, Steffen Klassert, linux-crypto, stable
[-- Attachment #1: Type: text/plain, Size: 2963 bytes --]
On Tue, 2020-05-19 at 16:00 -0400, Daniel Jordan wrote:
> Hello Ben,
>
> On Tue, May 19, 2020 at 02:53:05PM +0100, Ben Hutchings wrote:
> > I noticed that commit 07928d9bfc81 "padata: Remove broken queue
> > flushing" has been backported to most stable branches, but commit
> > 6fc4dbcf0276 "padata: Replace delayed timer with immediate workqueue in
> > padata_reorder" has not.
> >
> > Is this correct? What prevents the parallel_data ref-count from
> > dropping to 0 while the timer is scheduled?
>
> Doesn't seem like anything does, looking at 4.19.
OK, so it looks like the following commits should be backported:
[3.16-4.9] 119a0798dc42 padata: Remove unused but set variables
[3.16] de5540d088fe padata: avoid race in reordering
[3.16-4.9] 69b348449bda padata: get_next is never NULL
[3.16-4.14] cf5868c8a22d padata: ensure the reorder timer callback runs on the correct CPU
[3.16-4.14] 350ef88e7e92 padata: ensure padata_do_serial() runs on the correct CPU
[3.16-4.19] 6fc4dbcf0276 padata: Replace delayed timer with immediate workqueue in padata_reorder
[3.16-4.19] ec9c7d19336e padata: initialize pd->cpu with effective cpumask
[3.16-4.19] 065cf577135a padata: purge get_cpu and reorder_via_wq from padata_do_serial
Ben.
> I can see a race where the timer function uses a parallel_data after free
> whether or not the refcount goes to 0. Don't think it's likely to happen in
> practice because of how small the window is between the serial callback
> finishing and the timer being deactivated.
>
>
> task1:
> padata_reorder
> task2:
> padata_do_serial
> // object arrives in reorder queue
> // sees reorder_objects > 0,
> // set timer for 1 second
> mod_timer
> return
> padata_reorder
> // queue serial work, which finishes
> // (now possibly no more objects
> // left)
> |
> task1: |
> // pd is freed one of two ways: |
> // 1) pcrypt is unloaded |
> // 2) padata_replace triggered |
> // from userspace | (small window)
> |
> task3: |
> padata_reorder_timer |
> // uses pd after free |
> |
> del_timer // too late
>
>
> If I got this right we might want to backport the commit you mentioned to be on
> the safe side.
--
Ben Hutchings
All the simple programs have been written, and all the good names taken
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: Backporting "padata: Remove broken queue flushing"
2020-05-20 14:33 ` Ben Hutchings
@ 2020-05-21 8:00 ` Greg Kroah-Hartman
2020-05-21 13:32 ` Daniel Jordan
0 siblings, 1 reply; 5+ messages in thread
From: Greg Kroah-Hartman @ 2020-05-21 8:00 UTC (permalink / raw)
To: Ben Hutchings
Cc: Daniel Jordan, Sasha Levin, Herbert Xu, Steffen Klassert,
linux-crypto, stable
On Wed, May 20, 2020 at 03:33:44PM +0100, Ben Hutchings wrote:
> On Tue, 2020-05-19 at 16:00 -0400, Daniel Jordan wrote:
> > Hello Ben,
> >
> > On Tue, May 19, 2020 at 02:53:05PM +0100, Ben Hutchings wrote:
> > > I noticed that commit 07928d9bfc81 "padata: Remove broken queue
> > > flushing" has been backported to most stable branches, but commit
> > > 6fc4dbcf0276 "padata: Replace delayed timer with immediate workqueue in
> > > padata_reorder" has not.
> > >
> > > Is this correct? What prevents the parallel_data ref-count from
> > > dropping to 0 while the timer is scheduled?
> >
> > Doesn't seem like anything does, looking at 4.19.
>
> OK, so it looks like the following commits should be backported:
>
> [3.16-4.9] 119a0798dc42 padata: Remove unused but set variables
> [3.16] de5540d088fe padata: avoid race in reordering
> [3.16-4.9] 69b348449bda padata: get_next is never NULL
> [3.16-4.14] cf5868c8a22d padata: ensure the reorder timer callback runs on the correct CPU
> [3.16-4.14] 350ef88e7e92 padata: ensure padata_do_serial() runs on the correct CPU
These all applied cleanly to the needed trees, but these:
> [3.16-4.19] 6fc4dbcf0276 padata: Replace delayed timer with immediate workqueue in padata_reorder
> [3.16-4.19] ec9c7d19336e padata: initialize pd->cpu with effective cpumask
> [3.16-4.19] 065cf577135a padata: purge get_cpu and reorder_via_wq from padata_do_serial
Need some non-trivial backporting. Can you, or someone else do it so I
can queue them up? I don't have the free time at the moment, sorry.
thanks,
greg k-h
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: Backporting "padata: Remove broken queue flushing"
2020-05-21 8:00 ` Greg Kroah-Hartman
@ 2020-05-21 13:32 ` Daniel Jordan
0 siblings, 0 replies; 5+ messages in thread
From: Daniel Jordan @ 2020-05-21 13:32 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: Ben Hutchings, Daniel Jordan, Sasha Levin, Herbert Xu,
Steffen Klassert, linux-crypto, stable
On Thu, May 21, 2020 at 10:00:46AM +0200, Greg Kroah-Hartman wrote:
> but these:
>
> > [3.16-4.19] 6fc4dbcf0276 padata: Replace delayed timer with immediate workqueue in padata_reorder
> > [3.16-4.19] ec9c7d19336e padata: initialize pd->cpu with effective cpumask
> > [3.16-4.19] 065cf577135a padata: purge get_cpu and reorder_via_wq from padata_do_serial
>
> Need some non-trivial backporting. Can you, or someone else do it so I
> can queue them up? I don't have the free time at the moment, sorry.
Sure, I'll do these three.
Daniel
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2020-05-21 13:32 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-05-19 13:53 Backporting "padata: Remove broken queue flushing" Ben Hutchings
2020-05-19 20:00 ` Daniel Jordan
2020-05-20 14:33 ` Ben Hutchings
2020-05-21 8:00 ` Greg Kroah-Hartman
2020-05-21 13:32 ` Daniel Jordan
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).