staging: lustre: Remove VLA usage
PATCH v2 1/1 staging: lustre: Remove VLA usage
The kernel would like to have all stack VLA usage removed[1]. This switches to a simple kasprintf() instead, and in the process fixes an off-by-one between the allocation and the sprintf (allocation did not include NULL byte in calculation).
[1] https://lkml.org/lkml/2018/3/7/621
Signed-off-by Kees Cook <[email protected]>
Reviewed-by Rasmus Villemoes <[email protected]>
--- drivers/staging/lustre/lustre/llite/xattr.c | 19 +++++++++++++------ 1 file changed, 13 insertions(+), 6 deletions(-)
drivers/staging/lustre/lustre/llite/xattr.c +13 -6
@@ -87,10 +87,10 @@ ll_xattr_set_common(const struct xattr_handler *handler, const char *name, const void *value, size_t size, int flags) { - char fullname[strlen(handler->prefix) + strlen(name) + 1]; struct ll_sb_info *sbi = ll_i2sbi(inode);struct ptlrpc_request *req =NULL ;const char *pv = value; +char *fullname;__u64 valid;int rc;
@@ -141,10 +141,13 @@ ll_xattr_set_common(const struct xattr_handler *handler,return -EPERM ; } -sprintf (fullname,"%s%s\n" , handler->prefix, name); + fullname =kasprintf (GFP_KERNEL ,"%s%s\n" , handler->prefix, name); +if (!fullname) +return -ENOMEM ; rc =md_setxattr (sbi->ll_md_exp,ll_inode2fid (inode), valid, fullname, pv, size,0 , flags,ll_i2suppgid (inode), &req); +kfree (fullname);if (rc) {if (rc == -EOPNOTSUPP && handler->flags ==XATTR_USER_T ) {LCONSOLE_INFO ("Disabling user_xattr feature because it is not supported on the server\n" );
@@ -364,11 +367,11 @@ static int ll_xattr_get_common(const struct xattr_handler *handler, struct dentry *dentry, struct inode *inode, const char *name, void *buffer, size_t size) { - char fullname[strlen(handler->prefix) + strlen(name) + 1]; struct ll_sb_info *sbi = ll_i2sbi(inode);#ifdef CONFIG_FS_POSIX_ACL struct ll_inode_info *lli =ll_i2info (inode);#endif +char *fullname;int rc; CDEBUG(D_VFSTRACE, "VFS Op:inode=" DFID "(%p)\n",
@@ -411,9 +414,13 @@ static int ll_xattr_get_common(const struct xattr_handler *handler,if (handler->flags ==XATTR_ACL_DEFAULT_T && !S_ISDIR (inode->i_mode))return -ENODATA ;#endif -sprintf (fullname,"%s%s\n" , handler->prefix, name); -return ll_xattr_list (inode, fullname, handler->flags, buffer, size, -OBD_MD_FLXATTR ); + fullname =kasprintf (GFP_KERNEL ,"%s%s\n" , handler->prefix, name); +if (!fullname) +return -ENOMEM ; + rc =ll_xattr_list (inode, fullname, handler->flags, buffer, size, +OBD_MD_FLXATTR ); +kfree (fullname); +return rc; }static ssize_t ll_getxattr_lov (struct inode *inode,void *buf,size_t buf_size)
Signature · 6 lines
2.7.4 -- Kees Cook Pixel Security
Quoted 10 lines · Kees Cook
The kernel would like to have all stack VLA usage removed[1]. This switches to a simple
kasprintf()instead, and in the process fixes an off-by-one between the allocation and the sprintf (allocation did not include NULL byte in calculation).[1] https://lkml.org/lkml/2018/3/7/621
Signed-off-by Kees Cook <[email protected]>Reviewed-by Rasmus Villemoes <[email protected]>
This seems better than the VLA_SAFE() macro, at the cost of an extra kmalloc. I don't think these code paths are super performance critical.
Reviewed-by Andreas Dilger <[email protected]>
Quoted 76 lines
---
drivers/staging/lustre/lustre/llite/xattr.c| 19 +++++++++++++------1 file changed, 13 insertions(+), 6 deletions(-)drivers/staging/lustre/lustre/llite/xattr.c +13
-6@@ -87,10 +87,10 @@ ll_xattr_set_common(const struct xattr_handler *handler, const char *name, const void *value, size_t size, int flags) { - char fullname[strlen(handler->prefix) + strlen(name) + 1]; struct ll_sb_info *sbi = ll_i2sbi(inode);struct ptlrpc_request *req =NULL ;const char *pv = value; +char *fullname;__u64 valid;int rc;@@ -141,10 +141,13 @@ ll_xattr_set_common(const struct xattr_handler *handler,return -EPERM ; } -sprintf (fullname,"%s%s\n" , handler->prefix, name); + fullname =kasprintf (GFP_KERNEL ,"%s%s\n" , handler->prefix, name); +if (!fullname) +return -ENOMEM ; rc =md_setxattr (sbi->ll_md_exp,ll_inode2fid (inode), valid, fullname, pv, size,0 , flags,ll_i2suppgid (inode), &req); +kfree (fullname);if (rc) {if (rc == -EOPNOTSUPP && handler->flags ==XATTR_USER_T ) {LCONSOLE_INFO ("Disabling user_xattr feature because it is not supported on the server\n" );@@ -364,11 +367,11 @@ static int ll_xattr_get_common(const struct xattr_handler *handler, struct dentry *dentry, struct inode *inode, const char *name, void *buffer, size_t size) { - char fullname[strlen(handler->prefix) + strlen(name) + 1]; struct ll_sb_info *sbi = ll_i2sbi(inode);#ifdef CONFIG_FS_POSIX_ACL struct ll_inode_info *lli =ll_i2info (inode);#endif +char *fullname;int rc; CDEBUG(D_VFSTRACE, "VFS Op:inode=" DFID "(%p)\n",@@ -411,9 +414,13 @@ static int ll_xattr_get_common(const struct xattr_handler *handler,if (handler->flags ==XATTR_ACL_DEFAULT_T && !S_ISDIR (inode->i_mode))return -ENODATA ;#endif -sprintf (fullname,"%s%s\n" , handler->prefix, name); -return ll_xattr_list (inode, fullname, handler->flags, buffer, size, -OBD_MD_FLXATTR ); + fullname =kasprintf (GFP_KERNEL ,"%s%s\n" , handler->prefix, name); +if (!fullname) +return -ENOMEM ; + rc =ll_xattr_list (inode, fullname, handler->flags, buffer, size, +OBD_MD_FLXATTR ); +kfree (fullname); +return rc; }static ssize_t ll_getxattr_lov (struct inode *inode,void *buf,size_t buf_size)Signature · 10 lines
2.7.4 -- Kees Cook Pixel Security _______________________________________________ lustre-devel mailing list [email protected] http://lists.lustre.org/listinfo.cgi/lustre-devel-lustre.org
Cheers, Andreas
--
Andreas Dilger
Lustre Principal Architect
Intel Corporation