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:     Kees Cook <keescook@chromium.org>
CC:     Azeem Shaikh <azeemshaikh38@gmail.com>,
        Jeff Layton <jlayton@kernel.org>,
        "linux-hardening@vger.kernel.org" <linux-hardening@vger.kernel.org>,
        Neil Brown <neilb@suse.de>,
        Olga Kornievskaia <kolga@netapp.com>,
        Dai Ngo <dai.ngo@oracle.com>, Tom Talpey <tom@talpey.com>,
        Linux NFS Mailing List <linux-nfs@vger.kernel.org>,
        open list <linux-kernel@vger.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>,
        "netdev@vger.kernel.org" <netdev@vger.kernel.org>
Subject: Re: [PATCH] SUNRPC: Replace strlcpy with strscpy
Thread-Topic: [PATCH] SUNRPC: Replace strlcpy with strscpy
Thread-Index: AQHZnY+8OIo5sIDtZ0mcVtupqKlOma+IyJIAgABam4CAAABrgA==
Date:   Tue, 13 Jun 2023 19:43:44 +0000
Message-ID: <B3AC0B67-1629-44AC-8015-B28F020B018C@oracle.com>
References: <20230613004054.3539554-1-azeemshaikh38@gmail.com>
 <01E2FCED-7EB6-4D06-8BB0-FB0D141B546E@oracle.com>
 <202306131238.92CBED5@keescook>
In-Reply-To: <202306131238.92CBED5@keescook>
Accept-Language: en-US
Content-Language: en-US
x-ms-exchange-antispam-messagedata-chunkcount: 1
x-ms-exchange-antispam-messagedata-0: =?us-ascii?Q?HmJlJp5hL76wWJ6Lvlzewg0dGzAxAqQdZZgEo6s/+tu+4Ukxq9Kt9QAW+Ced?=
 =?us-ascii?Q?I2rmGkW1PKE154poXLcLtP9U+q+IUOYgAAoX9+wPUvnqPiCGre/zLc15zVB+?=
 =?us-ascii?Q?pjCp0kcVIrv1J3prdsRb+10zpCFWnMhX62gpxBwq9HkmtNi6rNUDGU7AniTK?=
 =?us-ascii?Q?7cJnwDpIXNtXL9SszjRq+BNngSLSGseEUPI40F7em/BhInPsSTyrdFmA5wJX?=
 =?us-ascii?Q?v9fNRj85PiHg2mIUoi3K9xl8xxx3azig9Wl5tk3cpmqNDSQoiZy+3upyxnia?=
 =?us-ascii?Q?GZpUi94IT7xjwCbP0RNcdFE6dQPycPdYg/N+pkPjeJ86hmAJ812OD3ZZ6r/0?=
 =?us-ascii?Q?5QK+MHmCiLIljKbCwZhTs8296Cn7Ng2kLH1TJKy9OE9JbHXppKbNfBgNUvoD?=
 =?us-ascii?Q?42MtX1uXNjpGqrYhdrUKdJa/Zg/lRcjyfQNbhWZODdYgoAWPViTjgTSESzS0?=
 =?us-ascii?Q?KO0l1amz+5jP8UkkLR/CTVLL6ev0N8wND16TQyeXTL/+3gQg86Yv3XOoVpNH?=
 =?us-ascii?Q?IOCCHEH4vp79qleeQrL4pj7M8wp5aVkhxwzSOEY3fQqVRElxszLSiwhOec0K?=
 =?us-ascii?Q?7g8oC9shLwpkHax/hZJocsAXnwYVp8zNdRdesixEBrbQ6usp8Jm9HlXA6r91?=
 =?us-ascii?Q?vq+keHtaLLdvGZoMB8fKtPlbXYhrFXOb9iKxSgJ534zJ5Sv59N0QRMAXfwdU?=
 =?us-ascii?Q?lB/vYaXyDH9oSzuyrHsF8q3OOvKWpa/zl9jpKqif4riCEfrdofFj5C/VM3S2?=
 =?us-ascii?Q?f0pKPxMl/SW2QZWlzGv0NA2SpTSJSVFlDREzgEPhbj2X8t2w3xWmgYWzX/Sy?=
 =?us-ascii?Q?ctKY/6Zxfz4SmmOKHlA0OjTafz1uXlfUKRXV52RYeiPaYLFK89hAG5gSXn/3?=
 =?us-ascii?Q?vM8W/CMmR69o4dw4darSvygVL9qJD199XoOT1aHucfF6ImBmv8DYH7M3uW4X?=
 =?us-ascii?Q?1b2plna0FfnxXlLBZj09nNVstGJxWrXoQx7psIpZtjL0WKCoCpXkK43WknSf?=
 =?us-ascii?Q?qX4TKOThQMCAwIJkgyL55A3Zt2NVvlTDe5wXosK7O8L1sxz7tIEHTLVusBcU?=
 =?us-ascii?Q?Fl3YfwEIWFVD0v3NWr+8ubSnCdIi/FC0AI2ve1rKCe5kM4FJfdzwGJSvbqxv?=
 =?us-ascii?Q?J7OJt4HtuBvbu9nzUdjU8UbOn+unl++PGKmsWebQxAGnrmMCH9I/Q+UGCOwC?=
 =?us-ascii?Q?LyutDXiWup9QywCp92KWQGYKqqPSEqRls8oXDkSEk+7JrUM2qFh47JHay1aC?=
 =?us-ascii?Q?+vRJOLTm+WQe8qHqMAMzTNUE1+ywb5A6Q6LcjIVxyK1X2kgEmt5P4MUVCue6?=
 =?us-ascii?Q?2XU6XoD9ay53qefePdDBF3yiOSJZJc7DRbp4Py7agvvj2D8bJqTbzDAKZ8cs?=
 =?us-ascii?Q?CdJGBiC1ArCVqeOjdZ0Lha9HhUJXd2XgipJ4APuBoVmq80zoRMpWO6qqq6a2?=
 =?us-ascii?Q?bM6PbmqnDGp32KWZUPasaw329Q0M3EWSRssLiaVI5xcpP6/0yyWrybb3JlgL?=
 =?us-ascii?Q?wrbVDAuqXAwnMV2I74C857RZmAkjoc3LAncRn3BbcGy9CmhWE+T0U1lkkizV?=
 =?us-ascii?Q?57oFrcF3ulY+kdXjjuBVxOlKXqzLPrjK1uDQIYdI3eRfyE523nXxS9XqI+gm?=
 =?us-ascii?Q?NQ=3D=3D?=
Content-Type: text/plain; charset="us-ascii"
Content-ID: <A57603D1FF00F948A1E920904FE23DE1@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?MUNzF7v8bcq32AXj/Ol9vQFHTX4YR/OY4csp0bjDChzN9lFzm891JI0/1/mz?=
 =?us-ascii?Q?KgTjcmvhbOojqMSqXBSq8oW7v5SYjcaTWSYCIBG4Dv8EFzGUoPfkJ8/X1KVL?=
 =?us-ascii?Q?qOjHlt2MCkZhkaWdb7MsvV4i8uhu9G4PRgr97s6HVhNm5ipcES1urxfnAchX?=
 =?us-ascii?Q?ldEs63u3xnOCAkZQAJFbdVANyhqEFJdYL/N6OXDF5UzSD3/iilWmdoel7rM8?=
 =?us-ascii?Q?g1AOPOq7y1QSX65f2BhLWlGXK+EWi6rcGZLjOantEhNiHC+wO3NJix2g+pRp?=
 =?us-ascii?Q?ix3+H6zDRPy/pYNx5MCXPoZhfg2cVlpfa1c5iWim+/iwRuOFiNS344NT5rAP?=
 =?us-ascii?Q?S6nBxhgJ38R7Abgy+vTPAG4WoE6vogkCmsVRjQEvAbYC2bKSyvwfPsOguR7Q?=
 =?us-ascii?Q?pJDfp6Exl6jMacsT+XB/dgmewDLPSb458RKFpaLQyFp1mm66G0R5zzqVQseW?=
 =?us-ascii?Q?kVr7RpUT5rbUSffY7sGJt3PRU+yjR8/KHMLWlQPpJlcxa8XPKvUzCONyrB4d?=
 =?us-ascii?Q?84e735zgaDrDnGxYclZL4B0vu3shpdGd/n/87fKDapyG+nZfROjQWl5MKD/S?=
 =?us-ascii?Q?Hu+wknvEclHDoQfQc3Nz9y+nB8h6PJwi6cz7aWM3oYz0e7zAuwC4hMQKbVIJ?=
 =?us-ascii?Q?Z/TrWz61HxQqpZ3oNH0zEX5y0gk9deWRFZ4MZ4IEneOqCPztgWjCqNbxIWDD?=
 =?us-ascii?Q?ZVca5TnVwG86PWSTm/booP1MfGi3t38kJibZZQyS7QTL1mgTFscteHRDjiA6?=
 =?us-ascii?Q?q65lKd1w6/qJmla2LfyHZdHzy2LGNfLh9cDmk55dcV9uRCQvpKG+JIVNZsmF?=
 =?us-ascii?Q?cCIFspQG2D6GxvYWU/Ot/BtuAewaMRUCSEJe9Jm9S/+R+utb4oEwLOzf1AW9?=
 =?us-ascii?Q?AsKJWABGyuZTm3P+Kt166v/p28T1istiUgEGdruB0E3SBRZLPwA05wdZs80D?=
 =?us-ascii?Q?Iup8KjOS7pdoa92BqnqiDOE+OmZfhVBDqKK/jeUbC8F4ucbJFB4ejUZ4QIpt?=
 =?us-ascii?Q?udlnkU0QeaxjWllNT0KhJMpNcfZ7ZjecbWwPGPnKy5zXOZElsMULEVYayZTN?=
 =?us-ascii?Q?CUc1OKfH?=
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: 353d0fc2-7ee3-45e8-4fb0-08db6c467f98
X-MS-Exchange-CrossTenant-originalarrivaltime: 13 Jun 2023 19:43:44.6715
 (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: 0gRVm0T+ADiTTMLoMJCKPNbbzN94VpvBR9FYCGi6xGyOJ6qaPLRS/f4+xj6ARTj6Yvpa9SmimF9shqzIRmFfcQ==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: SJ2PR10MB7558
X-Proofpoint-Virus-Version: vendor=baseguard
 engine=ICAP:2.0.254,Aquarius:18.0.957,Hydra:6.0.573,FMLib:17.11.176.26
 definitions=2023-06-13_22,2023-06-12_02,2023-05-22_02
X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 mlxlogscore=999 phishscore=0
 malwarescore=0 mlxscore=0 adultscore=0 bulkscore=0 spamscore=0
 suspectscore=0 classifier=spam adjust=0 reason=mlx scancount=1
 engine=8.12.0-2305260000 definitions=main-2306130172
X-Proofpoint-ORIG-GUID: -eNoWIHtt7ZmAMcWGVeVWVPFcbGjmlFV
X-Proofpoint-GUID: -eNoWIHtt7ZmAMcWGVeVWVPFcbGjmlFV
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,URIBL_BLOCKED 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 Jun 13, 2023, at 3:42 PM, Kees Cook <keescook@chromium.org> wrote:
>=20
> On Tue, Jun 13, 2023 at 02:18:06PM +0000, Chuck Lever III wrote:
>>=20
>>=20
>>> On Jun 12, 2023, at 8:40 PM, Azeem Shaikh <azeemshaikh38@gmail.com> wro=
te:
>>>=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().
>>=20
>> Using sprintf() seems cleaner to me: it would get rid of
>> the undocumented naked integer. Would that work for you?
>=20
> This is changing the "get" routine for reporting module parameters out
> of /sys. I think the right choice here is sysfs_emit(), as it performs
> the size tracking correctly. (Even the "default" sprintf() call should
> be replaced too, IMO.)

Agreed, that's even better.


>>> Direct replacement is safe here since the getter in kernel_params_ops
>>> handles -errorno return [3].
>>=20
>> s/errorno/errno/
>>=20
>>=20
>>> [1] https://www.kernel.org/doc/html/latest/process/deprecated.html#strl=
cpy
>>> [2] https://github.com/KSPP/linux/issues/89
>>> [3] https://elixir.bootlin.com/linux/v6.4-rc6/source/include/linux/modu=
leparam.h#L52
>>>=20
>>> Signed-off-by: Azeem Shaikh <azeemshaikh38@gmail.com>
>>> ---
>>> net/sunrpc/svc.c |    8 ++++----
>>> 1 file changed, 4 insertions(+), 4 deletions(-)
>>>=20
>>> diff --git a/net/sunrpc/svc.c b/net/sunrpc/svc.c
>>> index e6d4cec61e47..e5f379c4fdb3 100644
>>> --- a/net/sunrpc/svc.c
>>> +++ b/net/sunrpc/svc.c
>>> @@ -109,13 +109,13 @@ param_get_pool_mode(char *buf, const struct kerne=
l_param *kp)
>>> switch (*ip)
>>> {
>>> case SVC_POOL_AUTO:
>>> - return strlcpy(buf, "auto\n", 20);
>>> + return strscpy(buf, "auto\n", 20);
>=20
> e.g.
> return sysfs_emit(buf, "auto\n");
> ...
>=20
>>> case SVC_POOL_GLOBAL:
>>> - return strlcpy(buf, "global\n", 20);
>>> + return strscpy(buf, "global\n", 20);
>>> case SVC_POOL_PERCPU:
>>> - return strlcpy(buf, "percpu\n", 20);
>>> + return strscpy(buf, "percpu\n", 20);
>>> case SVC_POOL_PERNODE:
>>> - return strlcpy(buf, "pernode\n", 20);
>>> + return strscpy(buf, "pernode\n", 20);
>>> default:
>>> return sprintf(buf, "%d\n", *ip);
>=20
> and:
>=20
> return sysfs_emit(buf, "%d\n", *ip);
>=20
>=20
> -Kees
>=20
> --=20
> Kees Cook


--
Chuck Lever