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>,
        Kees Cook <keescook@chromium.org>,
        Linux NFS Mailing List <linux-nfs@vger.kernel.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH v2] NFSD: Remove all occurences of strlcpy
Thread-Topic: [PATCH v2] NFSD: Remove all occurences of strlcpy
Thread-Index: AQHZhOJYGV62rs3GkUCJMHqFG7AZcq9WvaEAgAAnyoA=
Date:   Fri, 12 May 2023 17:31:43 +0000
Message-ID: <5E054593-8995-46FF-824E-55B18BF53BE2@oracle.com>
References: <20230512145356.396567-1-azeemshaikh38@gmail.com>
 <109A2974-3AE0-48C6-8EE3-FADE95E8EF30@oracle.com>
In-Reply-To: <109A2974-3AE0-48C6-8EE3-FADE95E8EF30@oracle.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?VbIP3YMV7lX4STIJiP4nisXV2kaE3M6wsVYDZg0aqW06c41r1QyHmzg5/DaG?=
 =?us-ascii?Q?+rRY898dQ9iOQfYjShNxRxhjxKFL8eHAggtvuzwhF4SzEtAYHi35nmKB1sR0?=
 =?us-ascii?Q?YIqqbMcrwZb1agle/9rVoXRctWWb4SUvY47kvBLSnKW19yAQq4ygoKwzmNNa?=
 =?us-ascii?Q?8TWCf0GhhQ+gLgNz5R+JAhti6favF8DlL4E3agfBMvPaBFMDW8aGUaholIly?=
 =?us-ascii?Q?eZD5sbV1lL3DWBWsZX8VHe8Jpg/oEcoqpCZpVUFjgzv/PdUoEc3bdteRsKBf?=
 =?us-ascii?Q?N6eme7u6Vj5vctYLDfDHjJCq9lyj3S3IA79/MCli9PFf7nzHT0b5Qye1Fypi?=
 =?us-ascii?Q?phBmVaqLpMFICprsJkWegmTi0zFj+J2bQOF1bFdj1yEvZr2l5lEmD+LTu/Pz?=
 =?us-ascii?Q?7GOaThyriPF+4Y0hdujzaW4rE/hXCtND5F4OtMzsIwrgPY/Ufw/i7aTWd0EZ?=
 =?us-ascii?Q?CTdG6mfVG08i8Xgg4ciPhepwPzO1TTsSy9nRxUvwS0gVARxGZbiTWX4oB3LO?=
 =?us-ascii?Q?BthIPczhfGHwT2Pa6+eJ9+Vn9bOexObWoHWvKqVsTuL4mstj2yW0gx5sQOpO?=
 =?us-ascii?Q?tvVZP2Ra3bXoo+mYPFnFvZ1cf8I5ibWcxjnEX0xn4k8J1/OjmbzIs83Hn4JA?=
 =?us-ascii?Q?JKw96SvUTQMTsv37H8nLsilO0HQrEJGWaezO7hJH70VIoFlR70fkx1E6u6lM?=
 =?us-ascii?Q?hvV0J8H9BAFm1XS26NR+/Lphr4xJ2X8UFumQKUaX4flTYEXCyxa0D7IQ95ZR?=
 =?us-ascii?Q?BEA0e5cD15INU7TYDN8BW23MW2YgzW72SXazvrRkvm/B9k+SNnV/FULkgBEk?=
 =?us-ascii?Q?fzWHdgL+gYU+1L7Z2MVLOV8loMsN8WDVwZiTDBF3+R5WvPi7sO+7DYQdu55T?=
 =?us-ascii?Q?jAolI0XfaJkwv2VCvwjflD3QIbPwbPZPszjnmbnzU2YZVo9SfzgV11aq386g?=
 =?us-ascii?Q?6hz5qK4jlG33e2czbcDdOeIENQsfAsaIjAOiWF4iQjw6gCqVrzT/uLLvJa7W?=
 =?us-ascii?Q?r9UeAwVQ0BtcvbPUsGucJ0mrK/DD6XJ8gqwa/sfQ5bf4GhBW4tfcXezO543E?=
 =?us-ascii?Q?TxFBqBRe4PySBI7GbSp0uB5NQssjThT5BF09qXOrX21B/cX6WMmmYAbvW/Fj?=
 =?us-ascii?Q?RVa2UfzKkR93YkjTwn7P9xhsx4PH3O74MM/8CWTkxATOPHCfGaUXFiwZcRpP?=
 =?us-ascii?Q?QALZkyt0QbmOnZ0cS4+iB7QmOD/XeT+5Q7SJIAQ30uNTZHarF6VwAvCknJbX?=
 =?us-ascii?Q?4fa4rm2AQcI4rHCSFu6pkgm1EezBmJUQJR0WAGL3ACOjzpcPrbENRt1hkw/0?=
 =?us-ascii?Q?QC5Sre5kC0zsNTF84eT+Fq/Igc/vssK5K6S6BAaAU72ciQju8AsaN887Y+VQ?=
 =?us-ascii?Q?/F2dQSQPL1IpVw96+ElEo18GIJ37DE1mvhnmW7024josQi22fT8zFPAzuzpC?=
 =?us-ascii?Q?GCCdB58VHshEvdTn7o1bwWKrVp0ajfO9KY/sEFSzdeT64vZFADUl++zU3CtM?=
 =?us-ascii?Q?0KxnHYTc1jULhRhw9MsyK0NXLApe8DKaOpuO+3YKSSQ2TVrcvRltv8vf+jsX?=
 =?us-ascii?Q?fNOvm6soVVmw5PB+tEEMTBwMnvKM5Kdyr1xhY2YEFOXVXFVM7f0BJO/LO3JF?=
 =?us-ascii?Q?lA=3D=3D?=
Content-Type: text/plain; charset="us-ascii"
Content-ID: <00A2822B9AAEC1458262B14762722060@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: s1JQhZVCihGssfXMrZ+jq+xcCNP48EUBvjl+mH+Z7MvgS/aVbkQvIDxbRDj0mKyBttAZ0TObQXKDIn+F9iNRsDYJx7rj6AL9CZg+mXnRkFsRUNEpoNK/ZVMB6xqZwVrKSikeORqLLUOHhKPtz38isy6272J7Uomdx16iBqKjQu4e5qu7hR95N+rjr1KuYCC3/ovZirIRwTBHCioofS7XV2J/tUYWZOXlz3xCtBR1KftNvovtio84023RRkeMYlxPXbtmIiN0LJV9adnahxlxjExiSqq7i6bQ2m0Zsryr9H6mnN/avKQvlqRDKb6vIH/b8jdi0JEIiUTMY8Ozmz8rzsn8TNTkSTFmp/SPX5/DeZ5Ntig2OQQYMIimjQiAu+Ck62T0P0zN3yaDcjfR6BALQd/lYPBelQb8gsIQ9I0+Onq7t0P+e77mqxCycBA/HIfhIyrV/7ucOfC4o2zVuuv4GqCXJ8HL/y3JwDA1wvImCqFdtcrWi56so9mw0oJKKofbzgK05/9d88oZf53V7N0okn4PAsG9aicSNEzPWuWaq4UTkWFk+oleM6ZPjHdO1HXn91lTbPMlu088ClLilI+AJMNWTUtvOQTVT6PHPi0uIc23eciNeykNA3tMJQ4/H91dciOsf6Div53Dafvd8CZfmqWK/gQWUtx3j+oK85Ee9NoAuQAINcxMpnI1mmezBzrTpA2xBAl0rmiEBWg+OPpVbHUKw99f8lhsLqgATX6PA+BmvQtSICs7AQGdCWfjC+9WaF6EZ3N0bQIDOhGkwltS1pBKtYRacNZ5/jPPISPdfK3zij+h2VFr1/sz0JyQ1bXXbyyJrfGwvd1LDh8A7NCojC1STY1bsGqVmoJK4wS7UYM=
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: ec7d202a-b0a4-43ba-6e40-08db530ec147
X-MS-Exchange-CrossTenant-originalarrivaltime: 12 May 2023 17:31:43.9859
 (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: vR/28KCJwcyWn9h1iX4bN+CvzhMVzPh2uKR9QNHvSy26WXUbBbZhlGwKCKC/eWE1ckdewUyswQ5pa+6AZVZkvg==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: SJ0PR10MB4480
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-12_10,2023-05-05_01,2023-02-09_01
X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 mlxlogscore=999 adultscore=0 mlxscore=0
 bulkscore=0 suspectscore=0 malwarescore=0 spamscore=0 phishscore=0
 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2304280000
 definitions=main-2305120146
X-Proofpoint-GUID: iIawCqJUnSY8tESA1oZKrKa0jU9aI-RZ
X-Proofpoint-ORIG-GUID: iIawCqJUnSY8tESA1oZKrKa0jU9aI-RZ
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_H2,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 May 12, 2023, at 8:09 AM, Chuck Lever III <chuck.lever@oracle.com> wro=
te:
>=20
>=20
>=20
>> On May 12, 2023, at 7:53 AM, Azeem Shaikh <azeemshaikh38@gmail.com> wrot=
e:
>>=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.
>=20
> Let's update the patch description. This change is really
> a clean up -- it doesn't address the memory issues you
> originally described.

Unless, of course, you intend to apply this patch /after/
a patch that fixes __assign_str(). In that case, no change
to the patch description is needed.

Acked-by: Chuck Lever <chuck.lever@oracle.com <mailto:chuck.lever@oracle.co=
m>>


>> [1] https://www.kernel.org/doc/html/latest/process/deprecated.html#strlc=
py
>> [2] https://github.com/KSPP/linux/issues/89
>>=20
>> Fixes: 3c92fba557c6 ("NFSD: Enhance the nfsd_cb_setup tracepoint")
>> Signed-off-by: Azeem Shaikh <azeemshaikh38@gmail.com>
>> ---
>> v1: https://lore.kernel.org/all/20230510220952.3507366-1-azeemshaikh38@g=
mail.com/
>>=20
>> Changes in v2:
>> - Use __assign_str instead of strscpy.
>> - Added the Fixes tag.
>>=20
>> fs/nfsd/trace.h |    6 +++---
>> 1 file changed, 3 insertions(+), 3 deletions(-)
>>=20
>> diff --git a/fs/nfsd/trace.h b/fs/nfsd/trace.h
>> index 4183819ea082..72a906a053dc 100644
>> --- a/fs/nfsd/trace.h
>> +++ b/fs/nfsd/trace.h
>> @@ -1365,19 +1365,19 @@ TRACE_EVENT(nfsd_cb_setup,
>> __field(u32, cl_id)
>> __field(unsigned long, authflavor)
>> __sockaddr(addr, clp->cl_cb_conn.cb_addrlen)
>> - __array(unsigned char, netid, 8)
>> + __string(netid, netid)
>> ),
>> 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));
>> + __assign_str(netid, netid);
>> __entry->authflavor =3D authflavor;
>> __assign_sockaddr(addr, &clp->cl_cb_conn.cb_addr,
>> clp->cl_cb_conn.cb_addrlen)
>> ),
>> TP_printk("addr=3D%pISpc client %08x:%08x proto=3D%s flavor=3D%s",
>> __get_sockaddr(addr), __entry->cl_boot, __entry->cl_id,
>> - __entry->netid, show_nfsd_authflavor(__entry->authflavor))
>> + __get_str(netid), show_nfsd_authflavor(__entry->authflavor))
>> );
>>=20
>> TRACE_EVENT(nfsd_cb_setup_err,
>> --=20
>> 2.40.1.606.ga4b1b128d6-goog
>>=20
>>=20
>=20
> --
> Chuck Lever


--
Chuck Lever