On Wed, Jun 12, 2019 at 6:52 AM Yan, Zheng <ukernel@xxxxxxxxx> wrote: > > On Sat, Jun 8, 2019 at 2:47 AM Jeff Layton <jlayton@xxxxxxxxxx> 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@xxxxxxxxxx> > > Signed-off-by: Jeff Layton <jlayton@xxxxxxxxxx> > > --- > > 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