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:     "trondmy@kernel.org" <trondmy@kernel.org>
CC:     Linux NFS Mailing List <linux-nfs@vger.kernel.org>
Subject: Re: [PATCH 1/2] SUNRPC: Do not dereference non-socket transports in
 sysfs
Thread-Topic: [PATCH 1/2] SUNRPC: Do not dereference non-socket transports in
 sysfs
Thread-Index: AQHYP8e+sYOw+awWsEWGB/AApwPOn6zPEJGA
Date:   Thu, 24 Mar 2022 21:42:32 +0000
Message-ID: <80AE76C3-4D6D-4495-AD45-34EC73B12CEE@oracle.com>
References: <20220324213345.5833-1-trondmy@kernel.org>
In-Reply-To: <20220324213345.5833-1-trondmy@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?bNAk3zMqiusZulC/1lsO2ujV6s3bS0eUANmkjWaUyaAQT426bXlbx3sW+qsl?=
 =?us-ascii?Q?gKpWbzE7Enp3aKgaTP31qH0ZSmE8RsE1h6NL2tKxqlFNfycroeF7XKTG3xKS?=
 =?us-ascii?Q?McXHaoJVQd/jKUQwy1ANZSK0QByRHEJSLft72mLj4v0CYpxbOBqyePeFWThK?=
 =?us-ascii?Q?7XiHVX6rdiFz9SN/E/u/q9Xnypu1o0DawKryLkDiLsdJjGgFGS4YSa37wq2f?=
 =?us-ascii?Q?4IVzNZ8xo5cLavLRQwuk4e1MlYvkq9u0ta7V8ur+uJqh4mNLTdJ7uw6VfTBa?=
 =?us-ascii?Q?X79blTKsLiUfERH1PM+zkyIidZJKfH7bObBxtUpFEKWuiIKuon2mAOE2Bhno?=
 =?us-ascii?Q?Ta7B7dNh26in8EMbNLdeiWChNAdvyRlidl65+duPpLOashcfZekhAHKatcfs?=
 =?us-ascii?Q?qffvfXd9ihjJVREabAE9FuS+xoS8u21C9+tCHKARyPDbmxBIUHnuoNB/TCJ+?=
 =?us-ascii?Q?Qy93dVV6k/3FOQGceOLon5uF+gsaV72ulAY4xc7L+OAQsj1RSQPvq4FbT0aN?=
 =?us-ascii?Q?mb7dqbvAtpB145axmW4Bxs9z893CmHaGP2hc6Mckgo4ny4JQH6MDXNW2PTob?=
 =?us-ascii?Q?N3sxDKdDsVG9AREr7LLcozz8OUF2fkionJoIsSIaVX2NTd26LpU/kmcN1Cfo?=
 =?us-ascii?Q?eJuzGxxMIzeS4E9yI6nMgmlPMQRcQICHskrD3JQeMsoNei5xl0g889uGKKVs?=
 =?us-ascii?Q?jbmIlokvZ/FP9XnMrt7JF6JT6ditCOtE9WxCBbe++soYxMTWmJErLoCJG+PL?=
 =?us-ascii?Q?p8OifrmrbqfzOKeTwdx0fEsJO09YnAZowaxH+NYFVLS9w3nQVQthFMEJpik/?=
 =?us-ascii?Q?0I9czIFOOxEnzvg6XcErm9FrhjeuLNbEVkTt+m9RPjmbq7vBw6fZUsJ0tAym?=
 =?us-ascii?Q?K5QFLrb8EcFcOMuTlcpZaIgZfa/RZPxITQN3qweTDvJeCR0JpGyxiR2Dvdcf?=
 =?us-ascii?Q?Khpf6jVn6Zhbw/TwqarAgRqPIcGR7LyLRvS5hF2C87BD6QZoO5azkF4mDzHc?=
 =?us-ascii?Q?7RZAktRsJsCJ+bM7Np7uyLRPwfeHq1OqQGRwWmN9MtHkLjaebONwDZoHsGn5?=
 =?us-ascii?Q?rrkY6FTLC15IB0MGmIFXBKf6dVKaHP/YqpAarjI1RIQaTA3llXdCUjIfGrhy?=
 =?us-ascii?Q?ld47vbeYE9UpuS5dZ6yI8KYRd1BUF03xKYxqiLv+y9ffnfAWkpesWO00tK3D?=
 =?us-ascii?Q?MA5+HCGm2wyxnUB3yHQFtzvLoQMgNsyBB72vW26YS8nZITn8EqAO1TnJ1Ui6?=
 =?us-ascii?Q?IKUTacOyMaoafltP3u9zAu4aynRyrCK4cCjjbaj1shzD4Aw6RaUOxTx3kPAJ?=
 =?us-ascii?Q?ty/gBXZJpxGF+VjTqr/y5Nu4MPOee0s5BqjGJXhfAbkYdYc98mjdZxj/hTRV?=
 =?us-ascii?Q?tiFPUNfIpYN/DEoKYhjvU+pnoRQxVeVQRmyY8n9k6Soo/4293MYrlXL+jDC7?=
 =?us-ascii?Q?u34++/eEiFj8XQgHoUy0Zb+3ROqdMitR5gJkgA851p/nDiIJgqiC4A=3D=3D?=
Content-Type: text/plain; charset="us-ascii"
Content-ID: <9CBE828DC8284E47B87A2E071A100AB9@namprd10.prod.outlook.com>
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
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: 358acd66-6b90-4e84-44f5-08da0ddf3398
X-MS-Exchange-CrossTenant-originalarrivaltime: 24 Mar 2022 21:42:32.0206
 (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: wuvG0pVJxFCPKXtpDZRlCq7eXmeCiKBdV6BO8WMEeVjv1KK+2p8BnLV2PygcLpqXDvp9H20eiBHYTtGkN9uBxQ==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: MWHPR1001MB2096
X-Proofpoint-Virus-Version: vendor=nai engine=6300 definitions=10296 signatures=694973
X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 adultscore=0 spamscore=0 malwarescore=0
 suspectscore=0 mlxlogscore=999 bulkscore=0 phishscore=0 mlxscore=0
 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2202240000
 definitions=main-2203240113
X-Proofpoint-GUID: XRX5Jv-OakkBV-Pc6PuyCpbs-7bUcy9q
X-Proofpoint-ORIG-GUID: XRX5Jv-OakkBV-Pc6PuyCpbs-7bUcy9q
X-Spam-Status: No, score=-2.8 required=5.0 tests=BAYES_00,DKIM_SIGNED,
        DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,RCVD_IN_DNSWL_LOW,
        RCVD_IN_MSPIKE_H5,RCVD_IN_MSPIKE_WL,SPF_HELO_NONE,SPF_NONE,
        T_SCC_BODY_TEXT_LINE autolearn=ham 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


> On Mar 24, 2022, at 5:33 PM, trondmy@kernel.org wrote:
>=20
> From: Trond Myklebust <trond.myklebust@hammerspace.com>
>=20
> Do not cast the struct xprt to a sock_xprt unless we know it is a UDP or
> TCP transport. Otherwise the call to lock the mutex will scribble over
> whatever structure is actually there. This has been seen to cause hard
> system lockups when the underlying transport was RDMA.
>=20
> Fixes: e44773daf851 ("SUNRPC: Add srcaddr as a file in sysfs")
> Cc: stable@vger.kernel.org
> Signed-off-by: Trond Myklebust <trond.myklebust@hammerspace.com>
> ---
> net/sunrpc/sysfs.c | 32 ++++++++++++++++++++++----------
> 1 file changed, 22 insertions(+), 10 deletions(-)
>=20
> diff --git a/net/sunrpc/sysfs.c b/net/sunrpc/sysfs.c
> index 05c758da6a92..8ce053f84421 100644
> --- a/net/sunrpc/sysfs.c
> +++ b/net/sunrpc/sysfs.c
> @@ -107,22 +107,34 @@ static ssize_t rpc_sysfs_xprt_srcaddr_show(struct k=
object *kobj,
> 	struct rpc_xprt *xprt =3D rpc_sysfs_xprt_kobj_get_xprt(kobj);
> 	struct sockaddr_storage saddr;
> 	struct sock_xprt *sock;
> +	const char *fmt =3D "<closed>\n";
> 	ssize_t ret =3D -1;
>=20
> -	if (!xprt || !xprt_connected(xprt)) {
> -		xprt_put(xprt);
> -		return -ENOTCONN;
> -	}
> +	if (!xprt || !xprt_connected(xprt))
> +		goto out;
>=20
> -	sock =3D container_of(xprt, struct sock_xprt, xprt);
> -	mutex_lock(&sock->recv_mutex);
> -	if (sock->sock =3D=3D NULL ||
> -	    kernel_getsockname(sock->sock, (struct sockaddr *)&saddr) < 0)

This would be a little nicer if it went through the transport
switch instead. But isn't the socket address available in the
rpc_xprt?


> +	switch (xprt->xprt_class->ident) {
> +	case XPRT_TRANSPORT_UDP:
> +	case XPRT_TRANSPORT_TCP:
> +		break;
> +	default:
> +		fmt =3D "<not a socket>\n";
> 		goto out;
> +	};
>=20
> -	ret =3D sprintf(buf, "%pISc\n", &saddr);
> -out:
> +	sock =3D container_of(xprt, struct sock_xprt, xprt);
> +	mutex_lock(&sock->recv_mutex);
> +	if (sock->sock !=3D NULL) {
> +		ret =3D kernel_getsockname(sock->sock, (struct sockaddr *)&saddr);
> +		if (ret >=3D 0) {
> +			ret =3D sprintf(buf, "%pISc\n", &saddr);
> +			fmt =3D NULL;
> +		}
> +	}
> 	mutex_unlock(&sock->recv_mutex);
> +out:
> +	if (fmt)
> +		ret =3D sprintf(buf, fmt);
> 	xprt_put(xprt);
> 	return ret + 1;
> }
> --=20
> 2.35.1
>=20

--
Chuck Lever