From ab97ff746acd2a85c22c2116d450e95afb39d18a Mon Sep 17 00:00:00 2001 From: David Disseldorp Date: Mon, 15 Apr 2019 19:38:10 +0200 Subject: [PATCH] client: fix _listxattr() vxattr buffer length calculation Client::_listxattr() incorrectly returns a length based on the static _vxattrs_name_size() value. _vxattrs_calcu_name_size() only takes into account whether vxattrs are hidden, ignoring vxattr.exists_cb(). When filling the xattr buffer, Client::_listxattr() checks vxattr.hidden and vxattr.exists_cb(). When the latter returns false (as is the case for ceph.snap.btime on non-snapshots), we return a length which is larger than the amount that we wrote to the buffer. Fix this behaviour by always calculating the vxattrs length at runtime, taking both vxattr.hidden and vxattr.exists_cb() into account. Signed-off-by: David Disseldorp --- src/client/Client.cc | 74 ++++++++++++++++++++++++-------------------- 1 file changed, 41 insertions(+), 33 deletions(-) diff --git a/src/client/Client.cc b/src/client/Client.cc index dd60e1cf61435..a6ff9f7125868 100644 --- a/src/client/Client.cc +++ b/src/client/Client.cc @@ -11343,44 +11343,52 @@ int Client::ll_getxattr(Inode *in, const char *name, void *value, int Client::_listxattr(Inode *in, char *name, size_t size, const UserPerm& perms) { + bool len_only = (size == 0); int r = _getattr(in, CEPH_STAT_CAP_XATTR, perms, in->xattr_version == 0); - if (r == 0) { - for (map::iterator p = in->xattrs.begin(); - p != in->xattrs.end(); - ++p) - r += p->first.length() + 1; + if (r != 0) { + goto out; + } - const VXattr *vxattrs = _get_vxattrs(in); - r += _vxattrs_name_size(vxattrs); + r = 0; + for (const auto& p : in->xattrs) { + size_t this_len = p.first.length() + 1; + r += this_len; + if (len_only) + continue; - if (size != 0) { - if (size >= (unsigned)r) { - for (map::iterator p = in->xattrs.begin(); - p != in->xattrs.end(); - ++p) { - memcpy(name, p->first.c_str(), p->first.length()); - name += p->first.length(); - *name = '\0'; - name++; - } - if (vxattrs) { - for (int i = 0; !vxattrs[i].name.empty(); i++) { - const VXattr& vxattr = vxattrs[i]; - if (vxattr.hidden) - continue; - // call pointer-to-member function - if(vxattr.exists_cb && !(this->*(vxattr.exists_cb))(in)) - continue; - memcpy(name, vxattr.name.c_str(), vxattr.name.length()); - name += vxattr.name.length(); - *name = '\0'; - name++; - } - } - } else - r = -ERANGE; + if (this_len > size) { + r = -ERANGE; + goto out; } + + memcpy(name, p.first.c_str(), this_len); + name += this_len; + size -= this_len; } + + const VXattr *vxattr; + for (vxattr = _get_vxattrs(in); vxattr && !vxattr->name.empty(); vxattr++) { + if (vxattr->hidden) + continue; + // call pointer-to-member function + if (vxattr->exists_cb && !(this->*(vxattr->exists_cb))(in)) + continue; + + size_t this_len = vxattr->name.length() + 1; + r += this_len; + if (len_only) + continue; + + if (this_len > size) { + r = -ERANGE; + goto out; + } + + memcpy(name, vxattr->name.c_str(), this_len); + name += this_len; + size -= this_len; + } +out: ldout(cct, 8) << __func__ << "(" << in->ino << ", " << size << ") = " << r << dendl; return r; } -- 2.39.5