* [PATCH] sunrpc: only free unix grouplist after RCU settles
@ 2023-03-30 18:24 Jeff Layton
2023-03-30 18:31 ` Chuck Lever III
0 siblings, 1 reply; 4+ messages in thread
From: Jeff Layton @ 2023-03-30 18:24 UTC (permalink / raw)
To: chuck.lever; +Cc: linux-nfs, bcodding, Zhi Li
While the unix_gid object is rcu-freed, the group_info list that it
contains is not. Ensure that we only put the group list reference once
we are really freeing the unix_gid object.
Reported-by: Zhi Li <yieli@redhat.com>
Link: https://bugzilla.redhat.com/show_bug.cgi?id=2183056
Signed-off-by: Jeff Layton <jlayton@kernel.org>
---
net/sunrpc/svcauth_unix.c | 17 +++++++++++++----
1 file changed, 13 insertions(+), 4 deletions(-)
diff --git a/net/sunrpc/svcauth_unix.c b/net/sunrpc/svcauth_unix.c
index 50e2eb579194..4485088ce27b 100644
--- a/net/sunrpc/svcauth_unix.c
+++ b/net/sunrpc/svcauth_unix.c
@@ -416,14 +416,23 @@ static int unix_gid_hash(kuid_t uid)
return hash_long(from_kuid(&init_user_ns, uid), GID_HASHBITS);
}
-static void unix_gid_put(struct kref *kref)
+static void unix_gid_free(struct rcu_head *rcu)
{
- struct cache_head *item = container_of(kref, struct cache_head, ref);
- struct unix_gid *ug = container_of(item, struct unix_gid, h);
+ struct unix_gid *ug = container_of(rcu, struct unix_gid, rcu);
+ struct cache_head *item = &ug->h;
+
if (test_bit(CACHE_VALID, &item->flags) &&
!test_bit(CACHE_NEGATIVE, &item->flags))
put_group_info(ug->gi);
- kfree_rcu(ug, rcu);
+ kfree(ug);
+}
+
+static void unix_gid_put(struct kref *kref)
+{
+ struct cache_head *item = container_of(kref, struct cache_head, ref);
+ struct unix_gid *ug = container_of(item, struct unix_gid, h);
+
+ call_rcu(&ug->rcu, unix_gid_free);
}
static int unix_gid_match(struct cache_head *corig, struct cache_head *cnew)
--
2.39.2
^ permalink raw reply related [flat|nested] 4+ messages in thread* Re: [PATCH] sunrpc: only free unix grouplist after RCU settles
2023-03-30 18:24 [PATCH] sunrpc: only free unix grouplist after RCU settles Jeff Layton
@ 2023-03-30 18:31 ` Chuck Lever III
2023-03-30 18:49 ` Jeff Layton
0 siblings, 1 reply; 4+ messages in thread
From: Chuck Lever III @ 2023-03-30 18:31 UTC (permalink / raw)
To: Jeff Layton; +Cc: Linux NFS Mailing List, Benjamin Coddington, Zhi Li
Hi Jeff-
> On Mar 30, 2023, at 2:24 PM, Jeff Layton <jlayton@kernel.org> wrote:
>
> While the unix_gid object is rcu-freed, the group_info list that it
> contains is not. Ensure that we only put the group list reference once
> we are really freeing the unix_gid object.
>
> Reported-by: Zhi Li <yieli@redhat.com>
Should we also add
Fixes: fd5d2f78261b ("SUNRPC: Make server side AUTH_UNIX use lockless lookups") ?
> Link: https://bugzilla.redhat.com/show_bug.cgi?id=2183056
This bug isn't publicly accessible, fwiw.
> Signed-off-by: Jeff Layton <jlayton@kernel.org>
> ---
> net/sunrpc/svcauth_unix.c | 17 +++++++++++++----
> 1 file changed, 13 insertions(+), 4 deletions(-)
>
> diff --git a/net/sunrpc/svcauth_unix.c b/net/sunrpc/svcauth_unix.c
> index 50e2eb579194..4485088ce27b 100644
> --- a/net/sunrpc/svcauth_unix.c
> +++ b/net/sunrpc/svcauth_unix.c
> @@ -416,14 +416,23 @@ static int unix_gid_hash(kuid_t uid)
> return hash_long(from_kuid(&init_user_ns, uid), GID_HASHBITS);
> }
>
> -static void unix_gid_put(struct kref *kref)
> +static void unix_gid_free(struct rcu_head *rcu)
> {
> - struct cache_head *item = container_of(kref, struct cache_head, ref);
> - struct unix_gid *ug = container_of(item, struct unix_gid, h);
> + struct unix_gid *ug = container_of(rcu, struct unix_gid, rcu);
> + struct cache_head *item = &ug->h;
> +
> if (test_bit(CACHE_VALID, &item->flags) &&
> !test_bit(CACHE_NEGATIVE, &item->flags))
> put_group_info(ug->gi);
> - kfree_rcu(ug, rcu);
> + kfree(ug);
> +}
> +
> +static void unix_gid_put(struct kref *kref)
> +{
> + struct cache_head *item = container_of(kref, struct cache_head, ref);
> + struct unix_gid *ug = container_of(item, struct unix_gid, h);
> +
> + call_rcu(&ug->rcu, unix_gid_free);
> }
>
> static int unix_gid_match(struct cache_head *corig, struct cache_head *cnew)
> --
> 2.39.2
>
--
Chuck Lever
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH] sunrpc: only free unix grouplist after RCU settles
2023-03-30 18:31 ` Chuck Lever III
@ 2023-03-30 18:49 ` Jeff Layton
2023-03-30 19:33 ` Chuck Lever III
0 siblings, 1 reply; 4+ messages in thread
From: Jeff Layton @ 2023-03-30 18:49 UTC (permalink / raw)
To: Chuck Lever III; +Cc: Linux NFS Mailing List, Benjamin Coddington, Zhi Li
On Thu, 2023-03-30 at 18:31 +0000, Chuck Lever III wrote:
> Hi Jeff-
>
> > On Mar 30, 2023, at 2:24 PM, Jeff Layton <jlayton@kernel.org> wrote:
> >
> > While the unix_gid object is rcu-freed, the group_info list that it
> > contains is not. Ensure that we only put the group list reference once
> > we are really freeing the unix_gid object.
> >
> > Reported-by: Zhi Li <yieli@redhat.com>
>
> Should we also add
>
> Fixes: fd5d2f78261b ("SUNRPC: Make server side AUTH_UNIX use lockless lookups") ?
>
>
Sure. That does look like when that particular bug crept in.
> > Link: https://bugzilla.redhat.com/show_bug.cgi?id=2183056
>
> This bug isn't publicly accessible, fwiw.
>
Thanks. It should be now.
>
> > Signed-off-by: Jeff Layton <jlayton@kernel.org>
> > ---
> > net/sunrpc/svcauth_unix.c | 17 +++++++++++++----
> > 1 file changed, 13 insertions(+), 4 deletions(-)
> >
> > diff --git a/net/sunrpc/svcauth_unix.c b/net/sunrpc/svcauth_unix.c
> > index 50e2eb579194..4485088ce27b 100644
> > --- a/net/sunrpc/svcauth_unix.c
> > +++ b/net/sunrpc/svcauth_unix.c
> > @@ -416,14 +416,23 @@ static int unix_gid_hash(kuid_t uid)
> > return hash_long(from_kuid(&init_user_ns, uid), GID_HASHBITS);
> > }
> >
> > -static void unix_gid_put(struct kref *kref)
> > +static void unix_gid_free(struct rcu_head *rcu)
> > {
> > - struct cache_head *item = container_of(kref, struct cache_head, ref);
> > - struct unix_gid *ug = container_of(item, struct unix_gid, h);
> > + struct unix_gid *ug = container_of(rcu, struct unix_gid, rcu);
> > + struct cache_head *item = &ug->h;
> > +
> > if (test_bit(CACHE_VALID, &item->flags) &&
> > !test_bit(CACHE_NEGATIVE, &item->flags))
> > put_group_info(ug->gi);
> > - kfree_rcu(ug, rcu);
> > + kfree(ug);
> > +}
> > +
> > +static void unix_gid_put(struct kref *kref)
> > +{
> > + struct cache_head *item = container_of(kref, struct cache_head, ref);
> > + struct unix_gid *ug = container_of(item, struct unix_gid, h);
> > +
> > + call_rcu(&ug->rcu, unix_gid_free);
> > }
> >
> > static int unix_gid_match(struct cache_head *corig, struct cache_head *cnew)
> > --
> > 2.39.2
> >
>
> --
> Chuck Lever
>
>
--
Jeff Layton <jlayton@kernel.org>
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH] sunrpc: only free unix grouplist after RCU settles
2023-03-30 18:49 ` Jeff Layton
@ 2023-03-30 19:33 ` Chuck Lever III
0 siblings, 0 replies; 4+ messages in thread
From: Chuck Lever III @ 2023-03-30 19:33 UTC (permalink / raw)
To: Jeff Layton; +Cc: Linux NFS Mailing List, Benjamin Coddington, Zhi Li
> On Mar 30, 2023, at 2:49 PM, Jeff Layton <jlayton@kernel.org> wrote:
>
> On Thu, 2023-03-30 at 18:31 +0000, Chuck Lever III wrote:
>> Hi Jeff-
>>
>>> On Mar 30, 2023, at 2:24 PM, Jeff Layton <jlayton@kernel.org> wrote:
>>>
>>> While the unix_gid object is rcu-freed, the group_info list that it
>>> contains is not. Ensure that we only put the group list reference once
>>> we are really freeing the unix_gid object.
>>>
>>> Reported-by: Zhi Li <yieli@redhat.com>
>>
>> Should we also add
>>
>> Fixes: fd5d2f78261b ("SUNRPC: Make server side AUTH_UNIX use lockless lookups") ?
>>
>>
>
> Sure. That does look like when that particular bug crept in.
>
>>> Link: https://bugzilla.redhat.com/show_bug.cgi?id=2183056
>>
>> This bug isn't publicly accessible, fwiw.
>>
>
> Thanks. It should be now.
OK, applied to nfsd-fixes!
>>> Signed-off-by: Jeff Layton <jlayton@kernel.org>
>>> ---
>>> net/sunrpc/svcauth_unix.c | 17 +++++++++++++----
>>> 1 file changed, 13 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/net/sunrpc/svcauth_unix.c b/net/sunrpc/svcauth_unix.c
>>> index 50e2eb579194..4485088ce27b 100644
>>> --- a/net/sunrpc/svcauth_unix.c
>>> +++ b/net/sunrpc/svcauth_unix.c
>>> @@ -416,14 +416,23 @@ static int unix_gid_hash(kuid_t uid)
>>> return hash_long(from_kuid(&init_user_ns, uid), GID_HASHBITS);
>>> }
>>>
>>> -static void unix_gid_put(struct kref *kref)
>>> +static void unix_gid_free(struct rcu_head *rcu)
>>> {
>>> - struct cache_head *item = container_of(kref, struct cache_head, ref);
>>> - struct unix_gid *ug = container_of(item, struct unix_gid, h);
>>> + struct unix_gid *ug = container_of(rcu, struct unix_gid, rcu);
>>> + struct cache_head *item = &ug->h;
>>> +
>>> if (test_bit(CACHE_VALID, &item->flags) &&
>>> !test_bit(CACHE_NEGATIVE, &item->flags))
>>> put_group_info(ug->gi);
>>> - kfree_rcu(ug, rcu);
>>> + kfree(ug);
>>> +}
>>> +
>>> +static void unix_gid_put(struct kref *kref)
>>> +{
>>> + struct cache_head *item = container_of(kref, struct cache_head, ref);
>>> + struct unix_gid *ug = container_of(item, struct unix_gid, h);
>>> +
>>> + call_rcu(&ug->rcu, unix_gid_free);
>>> }
>>>
>>> static int unix_gid_match(struct cache_head *corig, struct cache_head *cnew)
>>> --
>>> 2.39.2
>>>
>>
>> --
>> Chuck Lever
>>
>>
>
> --
> Jeff Layton <jlayton@kernel.org>
--
Chuck Lever
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2023-03-30 19:33 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-03-30 18:24 [PATCH] sunrpc: only free unix grouplist after RCU settles Jeff Layton
2023-03-30 18:31 ` Chuck Lever III
2023-03-30 18:49 ` Jeff Layton
2023-03-30 19:33 ` Chuck Lever III
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox