All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2] staging: lustre: Remove VLA usage
@ 2018-03-07 20:54 ` Kees Cook
  0 siblings, 0 replies; 6+ messages in thread
From: Kees Cook @ 2018-03-07 20:54 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: devel, Dmitry Eremin, Tycho Andersen, Andreas Dilger,
	Kernel Hardening, Rasmus Villemoes, linux-kernel, Gargi Sharma,
	Oleg Drokin, Lustre Development List

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 <keescook@chromium.org>
Reviewed-by: Rasmus Villemoes <linux@rasmusvillemoes.dk>
---
 drivers/staging/lustre/lustre/llite/xattr.c | 19 +++++++++++++------
 1 file changed, 13 insertions(+), 6 deletions(-)

diff --git a/drivers/staging/lustre/lustre/llite/xattr.c b/drivers/staging/lustre/lustre/llite/xattr.c
index 532384c91447..ff6fe81a4ddb 100644
--- a/drivers/staging/lustre/lustre/llite/xattr.c
+++ b/drivers/staging/lustre/lustre/llite/xattr.c
@@ -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)
-- 
2.7.4


-- 
Kees Cook
Pixel Security
_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

^ permalink raw reply related	[flat|nested] 6+ messages in thread
* [lustre-devel] [PATCH v2] staging: lustre: Remove VLA usage
@ 2018-03-07 20:54 ` Kees Cook
  0 siblings, 0 replies; 6+ messages in thread
From: Kees Cook @ 2018-03-07 20:54 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: linux-kernel, Rasmus Villemoes, Tobin C. Harding, Tycho Andersen,
	Oleg Drokin, Andreas Dilger, James Simmons, Dmitry Eremin,
	Gargi Sharma, Lustre Development List, devel, Kernel Hardening

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 <keescook@chromium.org>
Reviewed-by: Rasmus Villemoes <linux@rasmusvillemoes.dk>
---
 drivers/staging/lustre/lustre/llite/xattr.c | 19 +++++++++++++------
 1 file changed, 13 insertions(+), 6 deletions(-)

diff --git a/drivers/staging/lustre/lustre/llite/xattr.c b/drivers/staging/lustre/lustre/llite/xattr.c
index 532384c91447..ff6fe81a4ddb 100644
--- a/drivers/staging/lustre/lustre/llite/xattr.c
+++ b/drivers/staging/lustre/lustre/llite/xattr.c
@@ -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)
-- 
2.7.4


-- 
Kees Cook
Pixel Security

^ permalink raw reply related	[flat|nested] 6+ messages in thread
* [PATCH v2] staging: lustre: Remove VLA usage
@ 2018-03-07 20:54 ` Kees Cook
  0 siblings, 0 replies; 6+ messages in thread
From: Kees Cook @ 2018-03-07 20:54 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: linux-kernel, Rasmus Villemoes, Tobin C. Harding, Tycho Andersen,
	Oleg Drokin, Andreas Dilger, James Simmons, Dmitry Eremin,
	Gargi Sharma, Lustre Development List, devel, Kernel Hardening

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 <keescook@chromium.org>
Reviewed-by: Rasmus Villemoes <linux@rasmusvillemoes.dk>
---
 drivers/staging/lustre/lustre/llite/xattr.c | 19 +++++++++++++------
 1 file changed, 13 insertions(+), 6 deletions(-)

diff --git a/drivers/staging/lustre/lustre/llite/xattr.c b/drivers/staging/lustre/lustre/llite/xattr.c
index 532384c91447..ff6fe81a4ddb 100644
--- a/drivers/staging/lustre/lustre/llite/xattr.c
+++ b/drivers/staging/lustre/lustre/llite/xattr.c
@@ -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)
-- 
2.7.4


-- 
Kees Cook
Pixel Security

^ permalink raw reply related	[flat|nested] 6+ messages in thread
* Re: [lustre-devel] [PATCH v2] staging: lustre: Remove VLA usage
  2018-03-07 20:54 ` [lustre-devel] " Kees Cook
  (?)
@ 2018-03-09  8:54   ` Dilger, Andreas
  -1 siblings, 0 replies; 6+ messages in thread
From: Dilger, Andreas @ 2018-03-09  8:54 UTC (permalink / raw)
  To: Kees Cook
  Cc: devel, Tycho Andersen, Kernel Hardening, Greg Kroah-Hartman,
	Rasmus Villemoes, linux-kernel, Gargi Sharma, Drokin, Oleg,
	Lustre Development List

On Mar 7, 2018, at 13:54, Kees Cook <keescook@chromium.org> wrote:
> 
> 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 <keescook@chromium.org>
> Reviewed-by: Rasmus Villemoes <linux@rasmusvillemoes.dk>

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 <andreas.dilger@intel.com>

> ---
> drivers/staging/lustre/lustre/llite/xattr.c | 19 +++++++++++++------
> 1 file changed, 13 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/staging/lustre/lustre/llite/xattr.c b/drivers/staging/lustre/lustre/llite/xattr.c
> index 532384c91447..ff6fe81a4ddb 100644
> --- a/drivers/staging/lustre/lustre/llite/xattr.c
> +++ b/drivers/staging/lustre/lustre/llite/xattr.c
> @@ -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)
> -- 
> 2.7.4
> 
> 
> -- 
> Kees Cook
> Pixel Security
> _______________________________________________
> lustre-devel mailing list
> lustre-devel@lists.lustre.org
> http://lists.lustre.org/listinfo.cgi/lustre-devel-lustre.org

Cheers, Andreas
--
Andreas Dilger
Lustre Principal Architect
Intel Corporation







_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

^ permalink raw reply	[flat|nested] 6+ messages in thread
* [lustre-devel] [PATCH v2] staging: lustre: Remove VLA usage
@ 2018-03-09  8:54   ` Dilger, Andreas
  0 siblings, 0 replies; 6+ messages in thread
From: Dilger, Andreas @ 2018-03-09  8:54 UTC (permalink / raw)
  To: Kees Cook
  Cc: Greg Kroah-Hartman, devel, Tycho Andersen, Kernel Hardening,
	Rasmus Villemoes, linux-kernel, Gargi Sharma, Drokin, Oleg,
	Lustre Development List, Tobin C. Harding

On Mar 7, 2018, at 13:54, Kees Cook <keescook@chromium.org> wrote:
> 
> 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 <keescook@chromium.org>
> Reviewed-by: Rasmus Villemoes <linux@rasmusvillemoes.dk>

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 <andreas.dilger@intel.com>

> ---
> drivers/staging/lustre/lustre/llite/xattr.c | 19 +++++++++++++------
> 1 file changed, 13 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/staging/lustre/lustre/llite/xattr.c b/drivers/staging/lustre/lustre/llite/xattr.c
> index 532384c91447..ff6fe81a4ddb 100644
> --- a/drivers/staging/lustre/lustre/llite/xattr.c
> +++ b/drivers/staging/lustre/lustre/llite/xattr.c
> @@ -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)
> -- 
> 2.7.4
> 
> 
> -- 
> Kees Cook
> Pixel Security
> _______________________________________________
> lustre-devel mailing list
> lustre-devel at lists.lustre.org
> http://lists.lustre.org/listinfo.cgi/lustre-devel-lustre.org

Cheers, Andreas
--
Andreas Dilger
Lustre Principal Architect
Intel Corporation

^ permalink raw reply	[flat|nested] 6+ messages in thread
* Re: [lustre-devel] [PATCH v2] staging: lustre: Remove VLA usage
@ 2018-03-09  8:54   ` Dilger, Andreas
  0 siblings, 0 replies; 6+ messages in thread
From: Dilger, Andreas @ 2018-03-09  8:54 UTC (permalink / raw)
  To: Kees Cook
  Cc: Greg Kroah-Hartman, devel, Tycho Andersen, Kernel Hardening,
	Rasmus Villemoes, linux-kernel, Gargi Sharma, Drokin, Oleg,
	Lustre Development List, Tobin C. Harding

On Mar 7, 2018, at 13:54, Kees Cook <keescook@chromium.org> wrote:
> 
> 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 <keescook@chromium.org>
> Reviewed-by: Rasmus Villemoes <linux@rasmusvillemoes.dk>

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 <andreas.dilger@intel.com>

> ---
> drivers/staging/lustre/lustre/llite/xattr.c | 19 +++++++++++++------
> 1 file changed, 13 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/staging/lustre/lustre/llite/xattr.c b/drivers/staging/lustre/lustre/llite/xattr.c
> index 532384c91447..ff6fe81a4ddb 100644
> --- a/drivers/staging/lustre/lustre/llite/xattr.c
> +++ b/drivers/staging/lustre/lustre/llite/xattr.c
> @@ -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)
> -- 
> 2.7.4
> 
> 
> -- 
> Kees Cook
> Pixel Security
> _______________________________________________
> lustre-devel mailing list
> lustre-devel@lists.lustre.org
> http://lists.lustre.org/listinfo.cgi/lustre-devel-lustre.org

Cheers, Andreas
--
Andreas Dilger
Lustre Principal Architect
Intel Corporation

^ permalink raw reply	[flat|nested] 6+ messages in thread
end of thread, other threads:[~2018-03-09  8:54 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-03-07 20:54 [PATCH v2] staging: lustre: Remove VLA usage Kees Cook
2018-03-07 20:54 ` Kees Cook
2018-03-07 20:54 ` [lustre-devel] " Kees Cook
2018-03-09  8:54 ` Dilger, Andreas
2018-03-09  8:54   ` Dilger, Andreas
2018-03-09  8:54   ` Dilger, Andreas

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.