On Sat, Jun 8, 2019 at 2:47 AM Jeff Layton <jlayton(a)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 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(a)redhat.com>
Signed-off-by: Jeff Layton <jlayton(a)kernel.org>
---
fs/ceph/xattr.c | 11 +++++++++--
1 file changed, 9 insertions(+), 2 deletions(-)
diff --git a/fs/ceph/xattr.c b/fs/ceph/xattr.c
index 6621d27e64f5..2a61e02e7166 100644
--- a/fs/ceph/xattr.c
+++ b/fs/ceph/xattr.c
@@ -803,8 +803,15 @@ 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))) {
+ /*
+ * getxattr_cb returns strlen(value), xattr length must
+ * include the NULL.
+ */
+ err = vxattr->getxattr_cb(ci, value, size) + 1;
This confuses getfattr. also causes failures of our test cases.
run getfattr without the patch, we get:
[root@kvm2-alpha ceph]# getfattr -n ceph.dir.entries .
# file: .
ceph.dir.entries="1"
with the patch, we get
[root@kvm1-alpha ceph]# getfattr -n ceph.dir.entries .
# file: .
ceph.dir.entries=0sMQA=
So the trick here is we need the user buffer to be large enough to
hold the NUL byte so we can use snprintf in the kernel. So I think if
the buffer is not large enough, return ERANGE or size+1 (NUL byte) if
value==NULL. If the buffer is large enough (size+1), then we return
size and not size+1.
Sound reasonable?
--
Patrick Donnelly, Ph.D.
He / Him / His
Senior Software Engineer
Red Hat Sunnyvale, CA
GPG: 19F28A586F808C2402351B93C3301A3E258DD79D