Received-SPF: pass (google.com: domain of linux-nfs-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
From:   Chuck Lever III <chuck.lever@oracle.com>
To:     Jeff Layton <jlayton@kernel.org>
CC:     Linux NFS Mailing List <linux-nfs@vger.kernel.org>,
        Benjamin Coddington <bcodding@redhat.com>,
        Zhi Li <yieli@redhat.com>
Subject: Re: [PATCH] sunrpc: only free unix grouplist after RCU settles
Thread-Topic: [PATCH] sunrpc: only free unix grouplist after RCU settles
Thread-Index: AQHZYzThgQcpowfBWk2wTdI98B7Wjq8TpTyA
Date:   Thu, 30 Mar 2023 18:31:22 +0000
Message-ID: <49AC9CF4-71A8-48D7-BF21-41FFFEFBE4C8@oracle.com>
References: <20230330182427.19013-1-jlayton@kernel.org>
In-Reply-To: <20230330182427.19013-1-jlayton@kernel.org>
Accept-Language: en-US
Content-Language: en-US
x-ms-exchange-antispam-messagedata-chunkcount: 1
x-ms-exchange-antispam-messagedata-0: =?us-ascii?Q?/cuqsSm/wxCpIJUjS/818CNpWtCQjGrt8hGthKBej1cZSMG6jRqMRzI0PeDv?=
 =?us-ascii?Q?gdli4H88yiH694/zJ92tstQ7RNWAcWzn3vAzK8VQUbs+qlYr86kk/N078JuM?=
 =?us-ascii?Q?8sg63dMKFJMA5qLci/reDWQxx2gyV88dnCGnoxZq2ubJ+M3ecaSCAyazU7hk?=
 =?us-ascii?Q?eEGt/utIkBHwOPtzRXrkC8UgI7OcYGq602i2eaSojNv8JPPtIxg7CCVCM85a?=
 =?us-ascii?Q?HTqB56+9bbKgS2Yoop4gMaVq6utV9jwjYZyXlkQkmV+Iz2oQNB/wrsEtBwrp?=
 =?us-ascii?Q?jsoMB/9y4h0Y+Vn+gpeqN/OWd7I+4Q0OuszKXN2L6ATUIUs8JV6OLQC8B/BS?=
 =?us-ascii?Q?QGnk7lvXrdKGb7silKQLq33JahLtH8UaJ00Mv4ti0Ut5XXVGPqL13RtsP3Hp?=
 =?us-ascii?Q?Copr4DgtcEznqJfWZ8S4iVoJfv3u57L6WIdJzT8SPvnl086w6hCP0abX+LvF?=
 =?us-ascii?Q?bKKRd/AOFq249WqO2aItvsQJ540iDqUnnKPBiPE32K4NdSTUV9IzUbWJa3PO?=
 =?us-ascii?Q?61p1d75IEsScndG9gBSuM+aEo4cVOoLxgdx1e2SljcrGMdv6NYBBhHU69pTm?=
 =?us-ascii?Q?TFU33drjX2yY+COtC69Yt6VJ/fWVYLVldFv8+PLp74w1kSi9TiMAizjFBCtN?=
 =?us-ascii?Q?b8rHB/xR4Yl48VV5eM2C/YfhV2f739eG/OEF+KINmCp/LAEWU5GLqOtadO97?=
 =?us-ascii?Q?40qKngSDDdV0VqNVXHXjtTdjWSdukCZeQTFAPwJP2NzzNjMoAV7rUsNEcn6z?=
 =?us-ascii?Q?c7bLOsL3pzx/GtE6j59cf718adUDKBchXwWocr44EFgbXR6EGr7oMDs4/x2I?=
 =?us-ascii?Q?CiJaAMmJPniCVhDReULsniDlyMEdAzJe3/HMivEAZA6b/yHNR4T3wiec2LW1?=
 =?us-ascii?Q?h5yhsn3mt2EE2NdCFyg2sqrIqa8wzuvvcNHK06OhanwBX6QjlH9mRxvG7MBz?=
 =?us-ascii?Q?giAUA/lS3gLpiveQRtGEijYoZl4VvkVS78OC22hw5IIrLPqP9T//YyCOqE+S?=
 =?us-ascii?Q?+4Lh6Bri3vZNuItKTHZXVFjmyuwhX14YVrrR/x0LfzvSQAqgwlTbtPJr/3jv?=
 =?us-ascii?Q?4vby4EBwE5JTrY3QlTTcSfdrNDQz1GaWDvcEwLAeBYWQ9IzlRIUnB3mFRxu9?=
 =?us-ascii?Q?meolzIphH5DYG1r+35jeuVIextqx/D+wOteij8E8Th6EVhDUIaI+EkAO/+K/?=
 =?us-ascii?Q?2b0HW2ZOXY5iELFxAV/3Q1XppBAX4q8Nz2M0I22S5Y66gAyFsD2Tz5/e7mDQ?=
 =?us-ascii?Q?1e1gAwoVkrCYjNXGuhN63/hl4rcVRi44ZuuwQesSRGePhcljt20Vl2UM9pfp?=
 =?us-ascii?Q?wt1LK3MvEzgs+IJ08GiOXjQZ1jGRItYWnJy2GEXsw8bxrU5/mQNE6Uf1zeiU?=
 =?us-ascii?Q?C9lQlpDjANtPF95O7gvBsFkJAXV2itLw3UKu2VlfAOt2uFQhBJRhAgwsmeDt?=
 =?us-ascii?Q?Kni3lwaR8rmZZoGXwX8b1O7svmP/XtR1iGf68EBcNMw2SiECltVi9wR0SYeN?=
 =?us-ascii?Q?4U4+485mmrRE3dBkuoPb7EtccaGXhgT4CR0XDf5eGOpDYMm40ZMJfZvq/3PN?=
 =?us-ascii?Q?hWyGNx/RRF9HmBe1Wjn2zZWM832WgIYqS0oHMO/VBy4uJftoRjgTZCSeKub4?=
 =?us-ascii?Q?Lw=3D=3D?=
Content-Type: text/plain; charset="us-ascii"
Content-ID: <795E7B06851CB2449CCF48C47AA7C7B5@namprd10.prod.outlook.com>
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-ExternalHop-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-ExternalHop-MessageData-0: F8PpCbuOGyd8GGMlILU5PJMcZzcdHYS/nJrVcyIzNOpM1eGuP+OpXQT6xjhArkdQKgarweIOR3JNw8mwGAQrf17dDCi5uqEqV5R8vUD95DwlkOv8bPaSIZ6ePf4LIA6+EjQ7DyhBtZZfeqJNlku7gAw39HX6qUYCSB76pdY6lqawe8tHAsA9Cm4Zd/LyRfS6/o50YhZ5A+D2jY+Fom/hEq7uiBBl8M2DQQiOrQstfizHkiC2eAjqn3T0D9Vfkr7Nv1bP/XJlz9X5wI+pX9O7ase/1Y7xHQhUi7gAC+A1AVVFiNgI8eOcyb7NJvEEYXoM1K194LRoPwctl8Bom8SxhBOf9rVSNFCh9L0q/yEVGvoVsEiLVCd17X4fbr5KK2YI9oFXWIuKXlj77okuZbZv6CkwKfK/Gd6Atm1gL8b5KV+hCujh/2mt/6/OGhPPWyn+BY1EyLSS2Z31Y2ooWUxtA3g5UfGnDbdI8h04khErEju5jkBhKGZmwuAv+kjvIBe1rnFZF8rJXy/gP1QIK/gURQl8j7wzhIiACbbp/0TE3h6XA0XGBk157VyWc5nFtB4itztg6+VfgkV8PQEmgK2WtslZvfL0mkz7o1FUHAKcyom5RhgLm8dDgHkigU14sPsA6XGWMniPx+Lj+z5S2cAXGDEK+oMgR8zKs5ne7d7yVMGCQnY9RXh3OfJJmlaEbkNEmVkjDdaYyrbLaLRJBTLM0hOpLQFOe+gciC8MNLmOVzFdx2Yw1dwbAPIvP9bu0H+/JUrXy9qJnQ6v+nUyXd7S24sHEUtSCFDrJmnbXlLYW59yKftzsKf6+/ZHsoqhzlY3
X-OriginatorOrg: oracle.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-AuthSource: BN0PR10MB5128.namprd10.prod.outlook.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 47995f15-0929-43b6-5b0d-08db314cf660
X-MS-Exchange-CrossTenant-originalarrivaltime: 30 Mar 2023 18:31:22.3415
 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Hosted
X-MS-Exchange-CrossTenant-id: 4e2c6054-71cb-48f1-bd6c-3a9705aca71b
X-MS-Exchange-CrossTenant-mailboxtype: HOSTED
X-MS-Exchange-CrossTenant-userprincipalname: jKSY9K2ydRRYmKN9v30cHg/lKRwXKV3+lm2YXhcnbOTUqu5GUkkjP3rSy/3Y1Guj0LWXG2BDBw7agVBdr5APOQ==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: SJ2PR10MB7656
X-Proofpoint-Virus-Version: vendor=baseguard
 engine=ICAP:2.0.254,Aquarius:18.0.942,Hydra:6.0.573,FMLib:17.11.170.22
 definitions=2023-03-30_11,2023-03-30_03,2023-02-09_01
X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 bulkscore=0 malwarescore=0
 suspectscore=0 mlxscore=0 mlxlogscore=999 spamscore=0 adultscore=0
 phishscore=0 classifier=spam adjust=0 reason=mlx scancount=1
 engine=8.12.0-2303200000 definitions=main-2303300146
X-Proofpoint-GUID: UI7cpQ5WXRp_zWMcwoZ50y-Y4GXl4ftF
X-Proofpoint-ORIG-GUID: UI7cpQ5WXRp_zWMcwoZ50y-Y4GXl4ftF
X-Spam-Status: No, score=-0.9 required=5.0 tests=DKIM_SIGNED,DKIM_VALID,
        DKIM_VALID_AU,DKIM_VALID_EF,RCVD_IN_DNSWL_LOW,RCVD_IN_MSPIKE_H2,
        SPF_HELO_NONE,SPF_NONE autolearn=unavailable autolearn_force=no
        version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on
        lindbergh.monkeyblade.net
Precedence: bulk
List-ID: <linux-nfs.vger.kernel.org>
X-Mailing-List: linux-nfs@vger.kernel.org

Hi Jeff-

> On Mar 30, 2023, at 2:24 PM, Jeff Layton <jlayton@kernel.org> wrote:
>=20
> 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.
>=20
> Reported-by: Zhi Li <yieli@redhat.com>

Should we also add

Fixes: fd5d2f78261b ("SUNRPC: Make server side AUTH_UNIX use lockless looku=
ps") ?


> Link: https://bugzilla.redhat.com/show_bug.cgi?id=3D2183056

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(-)
>=20
> 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);
> }
>=20
> -static void unix_gid_put(struct kref *kref)
> +static void unix_gid_free(struct rcu_head *rcu)
> {
> -	struct cache_head *item =3D container_of(kref, struct cache_head, ref);
> -	struct unix_gid *ug =3D container_of(item, struct unix_gid, h);
> +	struct unix_gid *ug =3D container_of(rcu, struct unix_gid, rcu);
> +	struct cache_head *item =3D &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 =3D container_of(kref, struct cache_head, ref);
> +	struct unix_gid *ug =3D container_of(item, struct unix_gid, h);
> +
> +	call_rcu(&ug->rcu, unix_gid_free);
> }
>=20
> static int unix_gid_match(struct cache_head *corig, struct cache_head *cn=
ew)
> --=20
> 2.39.2
>=20

--
Chuck Lever