Message ID | 20230831193827.1528867-1-azeemshaikh38@gmail.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Series | [v2] fs: ocfs2: Replace strlcpy with sysfs_emit | expand |
On Thu, Aug 31, 2023 at 07:38:27PM +0000, 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 <azeemshaikh38@gmail.com> Reviewed-by: Kees Cook <keescook@chromium.org>
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 <azeemshaikh38@gmail.com> Reviewed-by: Joseph Qi <joseph.qi@linux.alibaba.com> > --- > 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/20230830215426.4181755-1-azeemshaikh38@gmail.com/ > > 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 >
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);
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 <azeemshaikh38@gmail.com> --- 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/20230830215426.4181755-1-azeemshaikh38@gmail.com/ fs/ocfs2/dlmfs/dlmfs.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) -- 2.42.0.283.g2d96d420d3-goog