2023-09-13 03:51:10

by Joseph Qi

[permalink] [raw]
Subject: Re: [PATCH v2] fs: ocfs2: Replace strlcpy with sysfs_emit



On 9/1/23 3:38 AM, Azeem Shaikh wrote:
> 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 sysfs_emit().
>
> Direct replacement is safe here since its ok for `kernel_param_ops.get()`
> to return -errno [3].
>
> [1] https://www.kernel.org/doc/html/latest/process/deprecated.html#strlcpy
> [2] https://github.com/KSPP/linux/issues/89
> [3] https://elixir.bootlin.com/linux/v6.5/source/include/linux/moduleparam.h#L52
>
> Signed-off-by: Azeem Shaikh <[email protected]>

Reviewed-by: Joseph Qi <[email protected]>

> ---
> v2:
> * Removes patch [1/2] which replaced module_param_call with module_param_cb.
> * Use sysfs_emit instead of strscpy.
>
> v1:
> * https://lore.kernel.org/all/[email protected]/
>
> fs/ocfs2/dlmfs/dlmfs.c | 3 +--
> 1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/fs/ocfs2/dlmfs/dlmfs.c b/fs/ocfs2/dlmfs/dlmfs.c
> index 81265123ce6c..b38776ba3306 100644
> --- a/fs/ocfs2/dlmfs/dlmfs.c
> +++ b/fs/ocfs2/dlmfs/dlmfs.c
> @@ -80,8 +80,7 @@ static int param_set_dlmfs_capabilities(const char *val,
> static int param_get_dlmfs_capabilities(char *buffer,
> const struct kernel_param *kp)
> {
> - return strlcpy(buffer, DLMFS_CAPABILITIES,
> - strlen(DLMFS_CAPABILITIES) + 1);
> + return sysfs_emit(buffer, DLMFS_CAPABILITIES);
> }
> module_param_call(capabilities, param_set_dlmfs_capabilities,
> param_get_dlmfs_capabilities, NULL, 0444);
> --
> 2.42.0.283.g2d96d420d3-goog
>