Received-SPF: pass (google.com: domain of linux-nfs-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
From:   Chuck Lever III <chuck.lever@oracle.com>
To:     Bruce Fields <bfields@fieldses.org>,
        Jeff Layton <jlayton@kernel.org>
CC:     Dan Carpenter <dan.carpenter@oracle.com>,
        Linux NFS Mailing List <linux-nfs@vger.kernel.org>
Subject: Re: [bug report] nfsd: Protect session creation and client confirm
 using client_lock
Thread-Topic: [bug report] nfsd: Protect session creation and client confirm
 using client_lock
Thread-Index: AQHXo790A7Wd0tfa/0e3moKXgUhSZquYZ4qAgABDIACAAf4ZgIAAAbgA
Date:   Wed, 8 Sep 2021 21:32:15 +0000
Message-ID: <23A4CB30-F551-472F-9F2F-022C40AE1D70@oracle.com>
References: <20210907080732.GA20341@kili>
 <deba812574c9b898f99fc08f0c3fa23e85fc36ca.camel@kernel.org>
 <622EC724-ECBF-424D-A003-46A6B8E8C215@oracle.com>
 <20210908212605.GF23978@fieldses.org>
In-Reply-To: <20210908212605.GF23978@fieldses.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?9d0o1kQKqy+j1241+mx/E4PXjD0W1u/b+pOqvpne20+FKt9TAkfb1OE4GoZ6?=
 =?us-ascii?Q?e5OWbcQkJRsAsCVagV8K5Xu/2ZO21fAXf6i0uTVjtxaXr25ZtKm5ArsrTd51?=
 =?us-ascii?Q?DsHtIYvgyP68dQdxJCeDRrZbA0V+iF4OP7f+p4iny7z0km9Lu1vm4FhyhvGZ?=
 =?us-ascii?Q?LdWKKVgLbW6jbzSFZ8Km26o78Da5ALzRj+hW/oo/rpNy6D5pppf27gYiQlxE?=
 =?us-ascii?Q?m2W7nX+G2q8uGZmfXdEzrXOlBKZOoqV5WQZSqyZ1E2erLNomsyGuEJ0IXiFS?=
 =?us-ascii?Q?7i263gJN1wyR6rtIY8LVgRNkE0Md+CcrTHDEwlXAWNBE3X1D5GWjapZb8a7t?=
 =?us-ascii?Q?+lK08c87uTsBLq5St8QUBZ8HEEz108v7jBn/Iz4H12SaKX28PyRrVvPRstdH?=
 =?us-ascii?Q?XHifrzt4LhZDIaZqAvh0pX9C4sqW9FyUJWlG0dec2nx6XEWUSRnNJnXcBwh5?=
 =?us-ascii?Q?KEnU4nYSBAyZAwEG4AY6aXmlAG7wcYQWCYeVMzBw3j+kPt7hwRQZAdQM06aY?=
 =?us-ascii?Q?DCGYj+FAw1UKSoJoG0lDvW3RalYUhgqPYV4rUPzwJt24R5gA+hg8apiLMaAi?=
 =?us-ascii?Q?v7ZTnaXQb/wryD/IRK9FGvwz4+AXnh2NrE5zqjGx2vZ1vfufkU3ivGNfUiC+?=
 =?us-ascii?Q?MptGfAtg944HZgogW2jwDF90Kqbjp1hZ/TzbGNqerssfX5tVAUpG1aIq4DSe?=
 =?us-ascii?Q?6vk9PKZ3Ka1dDZaycRu6XmvwoaPrSN2P8aZ80TRhbvyX4rrX8RhZFRqQIgB+?=
 =?us-ascii?Q?8lm6DjtbqWQeqNEXvskwQ3OTfjzcZ3siBdTT7uQF/1SA4lqCPiDUvO1euqEe?=
 =?us-ascii?Q?eMjdZrmxVuMN46oh2smuIPG/DxoIx17DVEhEGoSu4BdYJDeshfwCc2deot4N?=
 =?us-ascii?Q?CDE6QQqCftavOp+cpnx439FSR8iTNnCjH8CaCGBs82faXlr60lVQgxHcNkMA?=
 =?us-ascii?Q?JgZ6UCogyaAlMbcPIEyGqmmeoJa2L3OeyKt/y7VpXGJPsikcwAqZljZouhth?=
 =?us-ascii?Q?jzdeq1G4Jy+r6effxDKE/cr6wPMOkydoGcn52Mo5dpQM7S8wocApkz9M6SJa?=
 =?us-ascii?Q?GxnLjGMP4gZ8G93lM4b9cbvDnJ46gI6in1sAwWqSDGNTXOcgSt04Y7c/2gOP?=
 =?us-ascii?Q?f0lX8Twm69vGnYX7jIsYocbj4wIV+o9myKOB1NSMHp6vrWpO/asc4KsM6pii?=
 =?us-ascii?Q?ydAqKfK4T54fjJoYXCbZYKo4JljbJqSH6RH0T6yzmF1qotqgKDHTSaUM97Nv?=
 =?us-ascii?Q?8dySngx3WsUhHy20c0+G3wlLGrHTg3dHkCOA+gL0jI/+krxIO5+H6f4oPGGv?=
 =?us-ascii?Q?0sLS3XDb+i97ajVCeIRgYeGR?=
Content-Type: text/plain; charset="us-ascii"
Content-ID: <94DF0B8BC45A3040A45B44DED186F115@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: SJ0PR10MB4688.namprd10.prod.outlook.com
X-MS-Exchange-CrossTenant-Network-Message-Id: f6f7f3e8-8a44-44aa-5a87-08d973102107
X-MS-Exchange-CrossTenant-originalarrivaltime: 08 Sep 2021 21:32:15.9749
 (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: UloIwHzdYyF/rvqK9zqHgzWGwgYcQr7xDjU++KMXMs/i0wcmpdTnfNN+eV3vjhGr4WyH2RjLWZmasyolPeyg1w==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: SJ0PR10MB4527
X-Proofpoint-Virus-Version: vendor=nai engine=6300 definitions=10101 signatures=668682
X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 bulkscore=0 suspectscore=0
 malwarescore=0 spamscore=0 mlxscore=0 phishscore=0 mlxlogscore=999
 adultscore=0 classifier=spam adjust=0 reason=mlx scancount=1
 engine=8.12.0-2109030001 definitions=main-2109080134
X-Proofpoint-GUID: OoXk6vNMV5kdyva6twK7ocSmgeZikZZT
X-Proofpoint-ORIG-GUID: OoXk6vNMV5kdyva6twK7ocSmgeZikZZT
Precedence: bulk
List-ID: <linux-nfs.vger.kernel.org>
X-Mailing-List: linux-nfs@vger.kernel.org


> On Sep 8, 2021, at 5:26 PM, Bruce Fields <bfields@fieldses.org> wrote:
>=20
> On Tue, Sep 07, 2021 at 03:00:23PM +0000, Chuck Lever III wrote:
>> We have IPV6_SCOPE_ID_LEN as a maximum size of the scope ID,
>> and it's not a big value. As long as boundary checking is made
>> to be sufficient, then a stack residency for the device name
>> should be safe.
>=20
> Something like this?  (Or are you making a patch?

I thought Jeff was going to handle it? More below.


> I'm not even sure how to test.)
>=20
> --b.
>=20
> diff --git a/net/sunrpc/addr.c b/net/sunrpc/addr.c
> index 6e4dbd577a39..d435bffc6199 100644
> --- a/net/sunrpc/addr.c
> +++ b/net/sunrpc/addr.c
> @@ -162,8 +162,10 @@ static int rpc_parse_scope_id(struct net *net, const=
 char *buf,
> 			      const size_t buflen, const char *delim,
> 			      struct sockaddr_in6 *sin6)
> {
> -	char *p;
> +	char p[IPV6_SCOPE_ID_LEN + 1];
> 	size_t len;
> +	u32 scope_id =3D 0;
> +	struct net_device *dev;
>=20
> 	if ((buf + buflen) =3D=3D delim)
> 		return 1;
> @@ -175,29 +177,23 @@ static int rpc_parse_scope_id(struct net *net, cons=
t char *buf,
> 		return 0;
>=20
> 	len =3D (buf + buflen) - delim - 1;
> -	p =3D kmemdup_nul(delim + 1, len, GFP_KERNEL);
> -	if (p) {
> -		u32 scope_id =3D 0;
> -		struct net_device *dev;
> -
> -		dev =3D dev_get_by_name(net, p);
> -		if (dev !=3D NULL) {
> -			scope_id =3D dev->ifindex;
> -			dev_put(dev);
> -		} else {
> -			if (kstrtou32(p, 10, &scope_id) !=3D 0) {
> -				kfree(p);
> -				return 0;
> -			}
> -		}
> -
> -		kfree(p);
> -
> -		sin6->sin6_scope_id =3D scope_id;
> -		return 1;
> +	if (len > IPV6_SCOPE_ID_LEN)
> +		return 0;
> +
> +	memcpy(p, delim + 1, len);
> +	p[len] =3D 0;

If I recall correctly, Linus prefers us to use the str*()
functions instead of raw memcpy() in cases like this.


> +
> +	dev =3D dev_get_by_name(net, p);
> +	if (dev !=3D NULL) {
> +		scope_id =3D dev->ifindex;
> +		dev_put(dev);
> +	} else {
> +		if (kstrtou32(p, 10, &scope_id) !=3D 0)
> +			return 0;
> 	}
>=20
> -	return 0;
> +	sin6->sin6_scope_id =3D scope_id;
> +	return 1;
> }
>=20
> static size_t rpc_pton6(struct net *net, const char *buf, const size_t bu=
flen,

--
Chuck Lever