| Message ID | 20231030011247.9794-1-neilb@suse.de (mailing list archive) |
|---|---|
| Headers | show |
| Series | sunrpc: not refcounting svc_serv | expand |
> On Oct 29, 2023, at 6:08 PM, NeilBrown <neilb@suse.de> wrote: > > This patch set continues earlier work of improving how threads and > services are managed. Specifically it drop the refcount. > > The refcount is always changed under the mutex, and almost always is > exactly equal to the number of threads. Those few cases where it is > more than the number of threads can usefully be handled other ways as > see in the patches. > > The first patches fixes a potential use-after-free when adding a socket > fails. This might be the UAF that Jeff mentioned recently. > > The second patch which removes the use of a refcount in pool_stats > handling is more complex than I would have liked, but I think it is > worth if for the result seen in 4/5 of substantial simplification. So I need a v2 of this series, then... - Move 4/5 to the beginning of the series (patch 1 or 2, doesn't matter) - Add R-b, Fixes, and Cc: stable tags to those two patches - Address Jeff's other comments AFAICT the 0-day bot reports were for the admin-revoked state series, not this one. -- Chuck Lever
On Wed, 01 Nov 2023, Chuck Lever III wrote: > > > On Oct 29, 2023, at 6:08 PM, NeilBrown <neilb@suse.de> wrote: > > > > This patch set continues earlier work of improving how threads and > > services are managed. Specifically it drop the refcount. > > > > The refcount is always changed under the mutex, and almost always is > > exactly equal to the number of threads. Those few cases where it is > > more than the number of threads can usefully be handled other ways as > > see in the patches. > > > > The first patches fixes a potential use-after-free when adding a socket > > fails. This might be the UAF that Jeff mentioned recently. > > > > The second patch which removes the use of a refcount in pool_stats > > handling is more complex than I would have liked, but I think it is > > worth if for the result seen in 4/5 of substantial simplification. > > So I need a v2 of this series, then... > > - Move 4/5 to the beginning of the series (patch 1 or 2, doesn't matter) I don't understand.... 2 and 3 are necessary prerequisites for 4. The remove places where the refcount is used. > > - Add R-b, Fixes, and Cc: stable tags to those two patches Will do > > - Address Jeff's other comments Yep. > > AFAICT the 0-day bot reports were for the admin-revoked state series, > not this one. Agreed. NeilBrown > > > -- > Chuck Lever > > >
> On Oct 31, 2023, at 1:40 PM, NeilBrown <neilb@suse.de> wrote: > > On Wed, 01 Nov 2023, Chuck Lever III wrote: >> >>> On Oct 29, 2023, at 6:08 PM, NeilBrown <neilb@suse.de> wrote: >>> >>> This patch set continues earlier work of improving how threads and >>> services are managed. Specifically it drop the refcount. >>> >>> The refcount is always changed under the mutex, and almost always is >>> exactly equal to the number of threads. Those few cases where it is >>> more than the number of threads can usefully be handled other ways as >>> see in the patches. >>> >>> The first patches fixes a potential use-after-free when adding a socket >>> fails. This might be the UAF that Jeff mentioned recently. >>> >>> The second patch which removes the use of a refcount in pool_stats >>> handling is more complex than I would have liked, but I think it is >>> worth if for the result seen in 4/5 of substantial simplification. >> >> So I need a v2 of this series, then... >> >> - Move 4/5 to the beginning of the series (patch 1 or 2, doesn't matter) > > I don't understand.... 2 and 3 are necessary prerequisites for 4. The > remove places where the refcount is used. Hrm. that's what I was afraid of. Isn't there a fix in 4/5 that we want applied to stable kernels, or did I misunderstand the email exchange between you and Jeff? -- Chuck Lever
On Wed, 01 Nov 2023, Chuck Lever III wrote: > > > On Oct 31, 2023, at 1:40 PM, NeilBrown <neilb@suse.de> wrote: > > > > On Wed, 01 Nov 2023, Chuck Lever III wrote: > >> > >>> On Oct 29, 2023, at 6:08 PM, NeilBrown <neilb@suse.de> wrote: > >>> > >>> This patch set continues earlier work of improving how threads and > >>> services are managed. Specifically it drop the refcount. > >>> > >>> The refcount is always changed under the mutex, and almost always is > >>> exactly equal to the number of threads. Those few cases where it is > >>> more than the number of threads can usefully be handled other ways as > >>> see in the patches. > >>> > >>> The first patches fixes a potential use-after-free when adding a socket > >>> fails. This might be the UAF that Jeff mentioned recently. > >>> > >>> The second patch which removes the use of a refcount in pool_stats > >>> handling is more complex than I would have liked, but I think it is > >>> worth if for the result seen in 4/5 of substantial simplification. > >> > >> So I need a v2 of this series, then... > >> > >> - Move 4/5 to the beginning of the series (patch 1 or 2, doesn't matter) > > > > I don't understand.... 2 and 3 are necessary prerequisites for 4. The > > remove places where the refcount is used. > > Hrm. that's what I was afraid of. > > Isn't there a fix in 4/5 that we want applied to stable kernels, > or did I misunderstand the email exchange between you and Jeff? > That is Commit bf32075256e9 ("NFSD: simplify error paths in nfsd_svc()") which is already in nfsd-next. NeilBrown > > -- > Chuck Lever > > >
> On Oct 31, 2023, at 1:46 PM, NeilBrown <neilb@suse.de> wrote: > > On Wed, 01 Nov 2023, Chuck Lever III wrote: >> >>> On Oct 31, 2023, at 1:40 PM, NeilBrown <neilb@suse.de> wrote: >>> >>> On Wed, 01 Nov 2023, Chuck Lever III wrote: >>>> >>>>> On Oct 29, 2023, at 6:08 PM, NeilBrown <neilb@suse.de> wrote: >>>>> >>>>> This patch set continues earlier work of improving how threads and >>>>> services are managed. Specifically it drop the refcount. >>>>> >>>>> The refcount is always changed under the mutex, and almost always is >>>>> exactly equal to the number of threads. Those few cases where it is >>>>> more than the number of threads can usefully be handled other ways as >>>>> see in the patches. >>>>> >>>>> The first patches fixes a potential use-after-free when adding a socket >>>>> fails. This might be the UAF that Jeff mentioned recently. >>>>> >>>>> The second patch which removes the use of a refcount in pool_stats >>>>> handling is more complex than I would have liked, but I think it is >>>>> worth if for the result seen in 4/5 of substantial simplification. >>>> >>>> So I need a v2 of this series, then... >>>> >>>> - Move 4/5 to the beginning of the series (patch 1 or 2, doesn't matter) >>> >>> I don't understand.... 2 and 3 are necessary prerequisites for 4. The >>> remove places where the refcount is used. >> >> Hrm. that's what I was afraid of. >> >> Isn't there a fix in 4/5 that we want applied to stable kernels, >> or did I misunderstand the email exchange between you and Jeff? >> > That is > Commit bf32075256e9 ("NFSD: simplify error paths in nfsd_svc()") > which is already in nfsd-next. OK. I'm still confused, but let's just keep the patch ordering the way it was in v1, then. We'll work out any missing fixes as they arise. -- Chuck Lever