On Fri, Jun 7, 2019 at 11:11 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 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@xxxxxxxxxx> > Signed-off-by: Jeff Layton <jlayton@xxxxxxxxxx> > --- > 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. -- Patrick Donnelly, Ph.D. He / Him / His Senior Software Engineer Red Hat Sunnyvale, CA GPG: 19F28A586F808C2402351B93C3301A3E258DD79D