From:   Chuck Lever III <chuck.lever@oracle.com>
To:     Benjamin Coddington <bcodding@redhat.com>
CC:     Jeff Layton <jlayton@kernel.org>,
        Trond Myklebust <trond.myklebust@hammerspace.com>,
        Anna Schumaker <anna@kernel.org>,
        "David S. Miller" <davem@davemloft.net>,
        Eric Dumazet <edumazet@google.com>,
        Jakub Kicinski <kuba@kernel.org>,
        Paolo Abeni <pabeni@redhat.com>,
        Bruce Fields <bfields@fieldses.org>,
        Linux NFS Mailing List <linux-nfs@vger.kernel.org>
Subject: Re: [PATCH 1/1] SUNRPC: Fix a server shutdown leak
Thread-Topic: [PATCH 1/1] SUNRPC: Fix a server shutdown leak
Thread-Index: AQHZThRbsBOmHSKc4EeHO2Q7T8KjwK7wO+SA
Date:   Wed, 8 Mar 2023 03:14:10 +0000
Message-ID: <FA64D05E-9902-4C37-9F14-40E3EEA3B656@oracle.com>
References: <cover.1677877233.git.bcodding@redhat.com>
 <65d0248533fbdea2f6190faa1ee150d2d615344b.1677877233.git.bcodding@redhat.com>
In-Reply-To: <65d0248533fbdea2f6190faa1ee150d2d615344b.1677877233.git.bcodding@redhat.com>
Accept-Language: en-US
Content-Language: en-US
x-ms-exchange-antispam-messagedata-chunkcount: 1
x-ms-exchange-antispam-messagedata-0: =?us-ascii?Q?yl/bocrQnSUknKhDV34NaOqNqLikoKttc46a/hPCGsXDSwLubWx9XAzm0VB+?=
 =?us-ascii?Q?i+2vnjTJoBzRLiA5OKy9B/rjsiq52fqM3SiPEXq0EmuDRVLAIsfnFk16k35U?=
 =?us-ascii?Q?UpjLe1tyKnUkArui+EMqU4TUSZyPeeGz0LQsED+2YqyABqP62Ddhj/TunB8C?=
 =?us-ascii?Q?dZsspw85eUzLDTV07yI5bZ5aCxW6l2jNyypUFzsY+JkAZsBOJXkPGKko3Jr7?=
 =?us-ascii?Q?W7X3D1SAilt5O5JwMslmZiCrBVhk1nE7qtXSU/AX2v0/ISvFwC/uJF31c8Jz?=
 =?us-ascii?Q?pIT/6bSDKTHYh4+dNj0oY18ZNY7NJKUy/VGBLgjzitG4IAi6h525uxOUgIMK?=
 =?us-ascii?Q?PsB/HAQPONs2l9yiTGomxItGWYs6T6JNjZuHTiq4ZZ9wCxR3d0MNebT/JXoa?=
 =?us-ascii?Q?FdNg1bjTddfhovu/lioJoOOM7r463+IfLhd4r6q33snaZVdKszj7rZhnnRHq?=
 =?us-ascii?Q?QPldaKWjLk8a03aOFua8oqOchCPUq51sorxr2QD9Xk2y1dtmO7MUcfI+uC+Y?=
 =?us-ascii?Q?skwhmCTeZ5fjN39LAlIRShu9XB9Plqkiw80QWP+bw+3F44e2txxR4q4ntjtk?=
 =?us-ascii?Q?iebrs6puer99SYue7odQVaevgCiTjFLkkMRLkxZWTQhIWCo9uLZkDvANscbt?=
 =?us-ascii?Q?Jez/sdfffPaK1wh35Fp/1vyGae84PWa9DEZOkoRBTM61R9US2rgYkX/GNDoB?=
 =?us-ascii?Q?nIpWquKGqyxyrFNzvtQZI5C3piEXuH4NwWIGyygFsogeLVrr6KCjABfx2hhT?=
 =?us-ascii?Q?08mJMUzks9+oCh3OgkvpeTFUBxdXOj9leQuFUmIPQdnpmpCY4WXaA9W+nuJc?=
 =?us-ascii?Q?nd1HTXnAcl+5+cX/sybXOF7tw2blshej0z7Jn6eHM95sA+YKeI0LWDEc/OSR?=
 =?us-ascii?Q?v0Ej8uhK6J/81n+/RoUQ4Iu9u2PbpHfWRC/UqGSlEJ7dP/hguKutvkhFBzMl?=
 =?us-ascii?Q?qS/vAfhQ+BsC4404qhLC8fdy5lQFykVFDatF+iBtQ835tmu/7WoZ17/jNXAq?=
 =?us-ascii?Q?7tDd3Xl6kYflqcrzcXn/oGbSdhnzFmXWPAIOwu/5Hr7ELejA8Wt/SKPytaD/?=
 =?us-ascii?Q?3Sr9YWvY2mncz6uW+ARXs0n9ZzkVrNTNq+DQX4F7uM7Bn1hUVETMeAdPtz8x?=
 =?us-ascii?Q?/dHh3BMQjb7UKn7omFIPWwINr75g+Rq/Qk2yNeFTxtE/JPiXgioYmnFBKIa5?=
 =?us-ascii?Q?VcCFeRhPj3UIsCdskZoa+5zBkK5AOspvFmwJ4eDPPmKezfxdpZW9kvHyzNdc?=
 =?us-ascii?Q?/RjNH0VQYedbWRBE9xBe+4MGCphs5fZZgtRRm009n1hFDim/VEck7DnerEJ+?=
 =?us-ascii?Q?FaijXMB6rox8bxnGUSOSqS6tf6MnAOMzEmqc2lMv+uf7/2blUMUV9eQNLIaY?=
 =?us-ascii?Q?Gm7dW+I1IyfD3TmU9Fa6R+Yf4DxkeLmUsHMOioVdTdYSD20JCEoK5cY7FdqT?=
 =?us-ascii?Q?+G9sEWeV6kJGimhBb47K3cUFu3yn7tLH42DBf1156PF6pNzlhqx8Euoif+Gk?=
 =?us-ascii?Q?yQXo8l4nygaTP6Qkcrh3sdW75tSZGRA3M1KTzTsDInLIDgZ0JjkIYeaOLgFK?=
 =?us-ascii?Q?ZxRyhqPeDoOZ4u2Tn74Vy7eck3MtZm8oH45jmQl84UlXufQZFhz+Trjb2VEE?=
 =?us-ascii?Q?RQ=3D=3D?=
Content-Type: text/plain; charset="us-ascii"
Content-ID: <ED3EE140D3E590469EF7663F1A0DF8AF@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: =?us-ascii?Q?ERYoT+g/rwAKOPz6P/V53BmaocLCCUaelJHXhtsKRjDLUIYygLiPTKymvxS4?=
 =?us-ascii?Q?W89JwHHP864NumFnTzwQ3MJmij7zzv4Gq+bJPTFesSRZZJ007gZBVOqWM4xL?=
 =?us-ascii?Q?r12xS51cq0m3k1WTpzDRTCZIGe65cilVQZ1MQeI39q+ohQBxmgcAax6zVT1i?=
 =?us-ascii?Q?n5p19XHO4uGrzC94ktVq9kX3czjK0+R6jRJsDqvUKArzM6gfjZ/qAQfJA3sV?=
 =?us-ascii?Q?2WLk9gUOw1sf7mY8w7wYTYFwQU+i6mPhTQz+Z8TAXuWjptvjBhxlTpXFcxas?=
 =?us-ascii?Q?6zRV7/aC9fsvhqtiAkJf6/hGzMJwEN/itjFY+t2bnOMi/6AGvIx9OgA4KdDO?=
 =?us-ascii?Q?gIjwyu0mPl5rZvrbva77GAOMl1tA/btrqboF3oZiwPZ+ya3oNjyLdu02E5Ms?=
 =?us-ascii?Q?p+Kdsu/6CVt+8ihKb6y+sWXfOMD4rtdQua4WFJMLQE8YfWN2z3u2ZiVxAzcI?=
 =?us-ascii?Q?OKwNAYBkXyNYJocoaEU5f46kOk6+HR9ugpPdUtO/oBDZnpFGM9rJWmJWCQDw?=
 =?us-ascii?Q?XBdpJ5Udb8+t3Y91vYGOvmfpn+FR/NPvoS+Uyphgk3gmNOIslIvOam82L/qG?=
 =?us-ascii?Q?7YUxTfrZyUyApK9PAgutrTl/i+dqwmkHaeHM9cOXfny60+w8XKMUQ6800Zzx?=
 =?us-ascii?Q?kUAOMXBZgSk2hpXN9ep6pwrhZItoYj98FXf+mG/oh7X21q6OeloO3bTKE7FX?=
 =?us-ascii?Q?DLJ9ynBk+FudTEDp9tv/gX121CKI4pTcLxfm+Dy3J5G52BqmslcR1iE4CwhD?=
 =?us-ascii?Q?hlHtVq9UdZuTv7VHCgfxwxRKUZIE/bftx15FF91kO/8tldJ3n5s1xsh8vbx7?=
 =?us-ascii?Q?nmd8Jfk2+a+F5BUdve97KwVTT+S/jugXQ45vUET60u0aXC4v3MyC+tHSEdqF?=
 =?us-ascii?Q?Zwtr8g4GMwbIEM1q7RLt1bA1bJ6B02cDF6T5wjHiexlXB+7eCqq1Soc7Q0M6?=
 =?us-ascii?Q?5SKu6E0a8v2ia9BigaNveg=3D=3D?=
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: 3a433243-f1ce-4de6-7e12-08db1f832f94
X-MS-Exchange-CrossTenant-originalarrivaltime: 08 Mar 2023 03:14:10.2026
 (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: zQ0cmtlz8e4Oy78rX1Q3zftWZZyg29SYdg2PpJzxFk18BCMWTHiucOzXR8sghaTGnhbnWwjMctglDwXMAjj+Ng==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: MN2PR10MB4206
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-07_18,2023-03-07_01,2023-02-09_01
X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 spamscore=0 malwarescore=0 mlxscore=0
 mlxlogscore=999 adultscore=0 suspectscore=0 phishscore=0 bulkscore=0
 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2212070000
 definitions=main-2303080027
X-Proofpoint-GUID: nnoTUMJ3ILWEbx6UzqiS67-z65btG7Fw
X-Proofpoint-ORIG-GUID: nnoTUMJ3ILWEbx6UzqiS67-z65btG7Fw
Precedence: bulk
List-ID: <linux-nfs.vger.kernel.org>
X-Mailing-List: linux-nfs@vger.kernel.org


> On Mar 3, 2023, at 4:08 PM, Benjamin Coddington <bcodding@redhat.com> wro=
te:
>=20
> Fix a race where kthread_stop() may prevent the threadfn from ever gettin=
g
> called.  If that happens the svc_rqst will not be cleaned up.
>=20
> Fixes: ed6473ddc704 ("NFSv4: Fix callback server shutdown")
> Signed-off-by: Benjamin Coddington <bcodding@redhat.com>
> ---
> net/sunrpc/svc.c | 6 +++++-
> 1 file changed, 5 insertions(+), 1 deletion(-)
>=20
> diff --git a/net/sunrpc/svc.c b/net/sunrpc/svc.c
> index 1fd3f5e57285..fea7ce8fba14 100644
> --- a/net/sunrpc/svc.c
> +++ b/net/sunrpc/svc.c
> @@ -798,6 +798,7 @@ svc_start_kthreads(struct svc_serv *serv, struct svc_=
pool *pool, int nrservs)
> static int
> svc_stop_kthreads(struct svc_serv *serv, struct svc_pool *pool, int nrser=
vs)
> {
> + struct svc_rqst *rqstp;
> struct task_struct *task;
> unsigned int state =3D serv->sv_nrthreads-1;
>=20
> @@ -806,7 +807,10 @@ svc_stop_kthreads(struct svc_serv *serv, struct svc_=
pool *pool, int nrservs)
> task =3D choose_victim(serv, pool, &state);
> if (task =3D=3D NULL)
> break;
> - kthread_stop(task);
> + rqstp =3D kthread_data(task);
> + /* Did we lose a race to svo_function threadfn? */
> + if (kthread_stop(task) =3D=3D -EINTR)
> + svc_exit_thread(rqstp);
> nrservs++;
> } while (nrservs < 0);
> return 0;
> --=20
> 2.31.1
>=20

Seems sensible, applied. Is there a bugzilla link that should be included?


--
Chuck Lever