ceph: fix getxattr return values for vxattrs
diff mbox series

Message ID 20190607181050.28085-1-jlayton@kernel.org
State New
Headers show
Series
  • ceph: fix getxattr return values for vxattrs
Related show

Commit Message

Jeff Layton June 7, 2019, 6:10 p.m. UTC
We have several virtual xattrs in cephfs which return various values as
strings. xattrs don't necessarily return strings however, so we need to
include the terminating NULL byte in the length when we return the
length.

Furthermore, the getxattr manpage says that we should return -ERANGE if
the buffer is too small to hold the resulting value. Let's start doing
that here as well.

URL: https://bugzilla.redhat.com/show_bug.cgi?id=1717454
Reported-by: Tomas Petr <tpetr@redhat.com>
Signed-off-by: Jeff Layton <jlayton@kernel.org>
---
 fs/ceph/xattr.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

Comments

Patrick Donnelly June 7, 2019, 6:28 p.m. UTC | #1
On Fri, Jun 7, 2019 at 11:11 AM Jeff Layton <jlayton@kernel.org> wrote:
>
> We have several virtual xattrs in cephfs which return various values as
> strings. xattrs don't necessarily return strings however, so we need to
> include the terminating NULL byte in the length when we return the
> length.
>
> Furthermore, the getxattr manpage says that we should return -ERANGE if
> the buffer is too small to hold the resulting value. Let's start doing
> that here as well.
>
> URL: https://bugzilla.redhat.com/show_bug.cgi?id=1717454
> Reported-by: Tomas Petr <tpetr@redhat.com>
> Signed-off-by: Jeff Layton <jlayton@kernel.org>
> ---
>  fs/ceph/xattr.c | 10 ++++++++--
>  1 file changed, 8 insertions(+), 2 deletions(-)
>
> diff --git a/fs/ceph/xattr.c b/fs/ceph/xattr.c
> index 6621d27e64f5..57f1bd83c21c 100644
> --- a/fs/ceph/xattr.c
> +++ b/fs/ceph/xattr.c
> @@ -803,8 +803,14 @@ ssize_t __ceph_getxattr(struct inode *inode, const char *name, void *value,
>                 if (err)
>                         return err;
>                 err = -ENODATA;
> -               if (!(vxattr->exists_cb && !vxattr->exists_cb(ci)))
> -                       err = vxattr->getxattr_cb(ci, value, size);
> +               if (!(vxattr->exists_cb && !vxattr->exists_cb(ci))) {
> +                       /* Make sure result will fit in buffer */
> +                       if (size > 0) {
> +                               if (size < vxattr->getxattr_cb(ci, NULL, 0) + 1)
> +                                       return -ERANGE;
> +                       }
>
> +                       err = vxattr->getxattr_cb(ci, value, size) + 1;

I don't think it's really necessary to call getxattr_cb twice here
when size>0. Otherwise LGTM.

Patch
diff mbox series

diff --git a/fs/ceph/xattr.c b/fs/ceph/xattr.c
index 6621d27e64f5..57f1bd83c21c 100644
--- a/fs/ceph/xattr.c
+++ b/fs/ceph/xattr.c
@@ -803,8 +803,14 @@  ssize_t __ceph_getxattr(struct inode *inode, const char *name, void *value,
 		if (err)
 			return err;
 		err = -ENODATA;
-		if (!(vxattr->exists_cb && !vxattr->exists_cb(ci)))
-			err = vxattr->getxattr_cb(ci, value, size);
+		if (!(vxattr->exists_cb && !vxattr->exists_cb(ci))) {
+			/* Make sure result will fit in buffer */
+			if (size > 0) {
+				if (size < vxattr->getxattr_cb(ci, NULL, 0) + 1)
+					return -ERANGE;
+			}
+			err = vxattr->getxattr_cb(ci, value, size) + 1;
+		}
 		return err;
 	}