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:     Azeem Shaikh <azeemshaikh38@gmail.com>
CC:     Jeff Layton <jlayton@kernel.org>,
        "linux-hardening@vger.kernel.org" <linux-hardening@vger.kernel.org>,
        Linux NFS Mailing List <linux-nfs@vger.kernel.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH] NFSD: Replace all non-returning strlcpy with strscpy
Thread-Topic: [PATCH] NFSD: Replace all non-returning strlcpy with strscpy
Thread-Index: AQHZg4w4w644hNPMzEqt/IwppCpl6q9VJ/yA
Date:   Thu, 11 May 2023 14:47:54 +0000
Message-ID: <72239648-C807-4CDD-8DA7-18440C83384E@oracle.com>
References: <20230510220952.3507366-1-azeemshaikh38@gmail.com>
In-Reply-To: <20230510220952.3507366-1-azeemshaikh38@gmail.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?ZHhNshjNOnomClncv+0qqJIZ1CQfdhGbIg1xZCW2m90yUYsF5sxuJN86cdaR?=
 =?us-ascii?Q?qGFAd8Pe1gUeEB7j9reckUB+PX5P4QAAZxLUpZkLbiF0uGIyV5tvHNFVMik8?=
 =?us-ascii?Q?n4MvmHKqjX+zMPh1gvXR0NwcRNPQKGylsvcm7W0/DaqX2nQA1l7vj7srKeD7?=
 =?us-ascii?Q?R19HS2w2O37V5m6zDZrF1V3E/UNbqCfMcLc/pDjGrk4nNAt05d3SjbmmQTjz?=
 =?us-ascii?Q?fBLIVKaY/yYgmqmKYb+viHaqiKhfF2pq2+WuK7//OX6TdLGlCae3AfMT6WX+?=
 =?us-ascii?Q?0TFnn9slFM9l120VdpIVMiewsXuOKM3QaB8xr31jPdJxOQ0ToLolmeHvwwPv?=
 =?us-ascii?Q?B39yMCrn0Bw2SNfyLoSYnqGHxsSHEQ6aSmgoyeknLd5ECq0Rv0VVgf1aOQzX?=
 =?us-ascii?Q?LVMBqmU3AK36443HQv0MFjuYXtLGv79SWGTLHRgWsofAJLSTiCFJ3pwF6YxM?=
 =?us-ascii?Q?JCu+7cz0GorbcX7+q1qajDFG52nML4u1EIMrzBnbhMwPh/VGzX9f2sP0dVTQ?=
 =?us-ascii?Q?VBYKA6sQu609klaE4MjkX9i8plu62ZFWaa7+mHualJTS0+On6L4j5+YSKCPq?=
 =?us-ascii?Q?n0rciju+u7M7GA6UHTjtadHAynlo4B3uE54ZnMZTYpTrPS5vlDcbQ4gqbCo/?=
 =?us-ascii?Q?iX6p3MCyig5ADRP7/aQicgujgSObWgZ3+TjPGO+x6JYEEa3MhW9LzEJNCQWm?=
 =?us-ascii?Q?xTMErll3jSnz/fXpPuXwZdIbdGy6yJRyWss+GPdaCnQJkaHkMssZM0z4B9L7?=
 =?us-ascii?Q?qwC52MPgqivLCQAbE0c05gUtUyytiuaRHycSJv7oFVdYPo5g3OYIbl8NATAp?=
 =?us-ascii?Q?DRJVtlCVGlovOGP+TxgbIyOEKsJZKsO7uV4/EkhaHcvhv18S6N3Nk3NEXIzv?=
 =?us-ascii?Q?BmAMV701oft92KSyENFBUR3RAFDrprJD5q7Hzy74eohSFg0PfTy6kCEQxihw?=
 =?us-ascii?Q?oXOMg77lpO8ovlShQAnW1oY3Je7qBYJjEkswV4dJo0CQY3MGyYgoblD3uZRA?=
 =?us-ascii?Q?1lt5C9Bsdd04pYNGYSDk6+m00eHvNLbo4gGlhB1WHFGyvKlJ5O+Sec8QF+6Z?=
 =?us-ascii?Q?g2ICCalBam3gVMNBMJxZgfqYjm6oWuZNr+dCAKKS8xhZT6SCJ1QsQoMnvBbd?=
 =?us-ascii?Q?xYwj3zi9iZj10J9iUmPytXwdqwf4fhsREHa9VE3AGm2G8rTGBrFHYSBn2NXR?=
 =?us-ascii?Q?aOqSRoWV6CXA5JgC/joKc+wBVPNlFCqH+zUeTMr3IsBtQh23SLvyQdxa2jYd?=
 =?us-ascii?Q?0WYJTpSVPv2+Mk0uZL+dqxA9pylPxARnmcmaQ05wVUNkjQRWxpwiuywuaDGI?=
 =?us-ascii?Q?3YI18XyPP51b1iih7ow95/pi5S6rthVFjGXVFGjtYiJ3ppXAVk2AY8e6xHKY?=
 =?us-ascii?Q?BIpcMsTy+9C0ObWZ3ub7PwVSaFlwUQ3w3aouCkhbGT3V7tS9vUu5U54HMHX/?=
 =?us-ascii?Q?PVOsQ1PhzWbPJ7WXKt1N6vsZ0zwGL6Ml907SM7v1IXIhQvuYmj7Gyj4lcwAj?=
 =?us-ascii?Q?SHtJe72Faw2mlKc0cEJ3QWzm6ToWa6c1yAhOq5Qc3bIVLyaBdU+ZzSwniaL4?=
 =?us-ascii?Q?PVoET2qpoaf+mfE0VUVJOR+wRolRwIokU1GXd8WepyeOLo0u6WD/TvhHKRGw?=
 =?us-ascii?Q?nA=3D=3D?=
Content-Type: text/plain; charset="us-ascii"
Content-ID: <F247C1E5C7D3774E8E98C246C3691EB5@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: /y6ks7mE4MKZJ7P0lqcxejCUjCfr/FDduIvZEqqydGEJoQoO8B9UTyAhI6qjznIKC8HKN5qtvXShCv1DRRA4KhVI6VEwPsBQk8/bg4dE3vOY0Qh5PXgAVsKjX6aXjP8D7CYEevbT/i1/ZKabpxp4bs4536KUWl/cUYEzSSptR/tH44YWtsxvpSFGykl3gI/CEwr1P5C0hZVRvDAtnsul7yE6blhT+Z8xgLlUBTfLSXCBY8nHLvIuQTCd8D/kHxltke/2dDWm5g2c7t2lltE9pGGStbLGe05s4/hCnyH74v9znH/XvLh9zJs/g78wQ46GKmWjQEVNIAicAD2M8Fq+/LeH6hQ7BdELHe+7+XTaZv5R+9MSyHKs81K3piwz8VSllVhk+Om1lCH3CRsEf+cVnoDnAGUTYIE1XSwP6v2s9sj0IVxTua+9lSrnVpCtuvp0L5atw+H6T/8Qg4DyyeJq32Zf825hgAO/7k+rSU1SEUsdv9EFR8fftUrUO3+lFzv68/CJIj63YwZ9snugXiFHkA7lFA0nkMPC4rtqHfA89PaXGF+6jzM+8EwXZLHnWiY46D79TEE0nzmqFyrz0boun+A4dUtJqcAxV6eFdCXEoOxPX6Lbmvjk1SppePUzcTTru6sCfxD3BiRzCh9SJ8MpEs1bs0PZwNVI71E9fbtUvXZVPbq16PUORpE7VKZtv7VcN+GSUjDHKOXMsNHdo4ZEtL2/vZ+rMnc9H6vGAsrNdboxuuTRWBKBWp+W6phmXvErVaU+7f1Uz3lgFcYRnScFYJIORCx1UIBXkWLdvssuS+9BgC7GYZbZm5aaz3fy+6Sb
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: 190e35a5-4b27-4849-9bb3-08db522eb41e
X-MS-Exchange-CrossTenant-originalarrivaltime: 11 May 2023 14:47:54.6472
 (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: wr2vM7buvdCCZn61SdY8zoopYP5zwJA+P/8/kQ/EvWLty45VLFU+bH3tu++27Hne5Pwz58WOe+6KkwDZA0DiYA==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: SJ1PR10MB5929
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-05-11_11,2023-05-05_01,2023-02-09_01
X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 mlxlogscore=931 adultscore=0
 suspectscore=0 malwarescore=0 phishscore=0 bulkscore=0 mlxscore=0
 spamscore=0 classifier=spam adjust=0 reason=mlx scancount=1
 engine=8.12.0-2304280000 definitions=main-2305110127
X-Proofpoint-GUID: xg9u6oicdVT7C4weFTkRI_gnHT7avbgR
X-Proofpoint-ORIG-GUID: xg9u6oicdVT7C4weFTkRI_gnHT7avbgR
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,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

Hello Azeem -

> On May 10, 2023, at 3:09 PM, Azeem Shaikh <azeemshaikh38@gmail.com> wrote=
:
>=20
> strlcpy() reads the entire source buffer first.
> This read may exceed the destination size limit.
> This is both inefficient and can lead to linear read
> overflows if a source string is not NUL-terminated [1].
> In an effort to remove strlcpy() completely [2], replace
> strlcpy() here with strscpy().
> No return values were used, so direct replacement is safe.

Actually netid should use the __string() and __assign_str()
macros rather than open-coding a string copy, I think.


> [1] https://www.kernel.org/doc/html/latest/process/deprecated.html#strlcp=
y
> [2] https://github.com/KSPP/linux/issues/89
>=20

I suggest adding:

Fixes: 3c92fba557c6 ("NFSD: Enhance the nfsd_cb_setup tracepoint")

> Signed-off-by: Azeem Shaikh <azeemshaikh38@gmail.com>
> ---
> fs/nfsd/trace.h |    2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>=20
> diff --git a/fs/nfsd/trace.h b/fs/nfsd/trace.h
> index 4183819ea082..9b32cda54808 100644
> --- a/fs/nfsd/trace.h
> +++ b/fs/nfsd/trace.h
> @@ -1370,7 +1370,7 @@ TRACE_EVENT(nfsd_cb_setup,
> TP_fast_assign(
> __entry->cl_boot =3D clp->cl_clientid.cl_boot;
> __entry->cl_id =3D clp->cl_clientid.cl_id;
> - strlcpy(__entry->netid, netid, sizeof(__entry->netid));
> + strscpy(__entry->netid, netid, sizeof(__entry->netid));
> __entry->authflavor =3D authflavor;
> __assign_sockaddr(addr, &clp->cl_cb_conn.cb_addr,
>  clp->cl_cb_conn.cb_addrlen)
>=20

--
Chuck Lever