From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
Map xattr names originating at the client; from get/set/remove xattr.
Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
---
tools/virtiofsd/passthrough_ll.c | 101 ++++++++++++++++++++++++++++++-
1 file changed, 98 insertions(+), 3 deletions(-)
diff --git a/tools/virtiofsd/passthrough_ll.c b/tools/virtiofsd/passthrough_ll.c
index f5a33014f9..57ebe17ed6 100644
--- a/tools/virtiofsd/passthrough_ll.c
+++ b/tools/virtiofsd/passthrough_ll.c
@@ -2183,20 +2183,80 @@ static XattrMapEntry *parse_xattrmap(struct lo_data *lo)
return res;
}
-static void lo_getxattr(fuse_req_t req, fuse_ino_t ino, const char *name,
+/*
+ * For use with getxattr/setxattr/removexattr, where the client
+ * gives us a name and we may need to choose a different one.
+ * Allocates a buffer for the result placing it in *out_name.
+ * If there's no change then *out_name is not set.
+ * Returns 0 on success
+ * Can return -EPERM to indicate we block a given attribute
+ * (in which case out_name is not allocated)
+ * Can return -ENOMEM to indicate out_name couldn't be allocated.
+ */
+static int xattr_map_client(const struct lo_data *lo, const char *client_name,
+ char **out_name)
+{
+ const XattrMapEntry *cur_entry;
+
+ for (cur_entry = lo->xattr_map_list; ; cur_entry++) {
+ if ((cur_entry->flags & XATTR_MAP_FLAG_CLIENT) &&
+ (strstart(client_name, cur_entry->key, NULL))) {
+ if (cur_entry->flags & XATTR_MAP_FLAG_END_BAD) {
+ return -EPERM;
+ }
+ if (cur_entry->flags & XATTR_MAP_FLAG_END_OK) {
+ /* Unmodified name */
+ return 0;
+ }
+ if (cur_entry->flags & XATTR_MAP_FLAG_PREFIX) {
+ *out_name = g_try_malloc(strlen(client_name) +
+ strlen(cur_entry->prepend) + 1);
+ if (!*out_name) {
+ return -ENOMEM;
+ }
+ sprintf(*out_name, "%s%s", cur_entry->prepend, client_name);
+ return 0;
+ }
+ }
+ }
+
+ /* Shouldn't get here - rules should have an END_* */
+ abort();
+}
+
+static void lo_getxattr(fuse_req_t req, fuse_ino_t ino, const char *in_name,
size_t size)
{
struct lo_data *lo = lo_data(req);
char *value = NULL;
char procname[64];
+ const char *name;
+ char *mapped_name;
struct lo_inode *inode;
ssize_t ret;
int saverr;
int fd = -1;
+ mapped_name = NULL;
+ name = in_name;
+ if (lo->xattrmap) {
+ ret = xattr_map_client(lo, in_name, &mapped_name);
+ if (ret < 0) {
+ if (ret == -EPERM) {
+ ret = -ENODATA;
+ }
+ fuse_reply_err(req, -ret);
+ return;
+ }
+ if (mapped_name) {
+ name = mapped_name;
+ }
+ }
+
inode = lo_inode(req, ino);
if (!inode) {
fuse_reply_err(req, EBADF);
+ g_free(mapped_name);
return;
}
@@ -2261,6 +2321,7 @@ out_err:
saverr = errno;
out:
fuse_reply_err(req, saverr);
+ g_free(mapped_name);
goto out_free;
}
@@ -2338,19 +2399,35 @@ out:
goto out_free;
}
-static void lo_setxattr(fuse_req_t req, fuse_ino_t ino, const char *name,
+static void lo_setxattr(fuse_req_t req, fuse_ino_t ino, const char *in_name,
const char *value, size_t size, int flags)
{
char procname[64];
+ const char *name;
+ char *mapped_name;
struct lo_data *lo = lo_data(req);
struct lo_inode *inode;
ssize_t ret;
int saverr;
int fd = -1;
+ mapped_name = NULL;
+ name = in_name;
+ if (lo->xattrmap) {
+ ret = xattr_map_client(lo, in_name, &mapped_name);
+ if (ret < 0) {
+ fuse_reply_err(req, -ret);
+ return;
+ }
+ if (mapped_name) {
+ name = mapped_name;
+ }
+ }
+
inode = lo_inode(req, ino);
if (!inode) {
fuse_reply_err(req, EBADF);
+ g_free(mapped_name);
return;
}
@@ -2385,21 +2462,38 @@ out:
}
lo_inode_put(lo, &inode);
+ g_free(mapped_name);
fuse_reply_err(req, saverr);
}
-static void lo_removexattr(fuse_req_t req, fuse_ino_t ino, const char *name)
+static void lo_removexattr(fuse_req_t req, fuse_ino_t ino, const char *in_name)
{
char procname[64];
+ const char *name;
+ char *mapped_name;
struct lo_data *lo = lo_data(req);
struct lo_inode *inode;
ssize_t ret;
int saverr;
int fd = -1;
+ mapped_name = NULL;
+ name = in_name;
+ if (lo->xattrmap) {
+ ret = xattr_map_client(lo, in_name, &mapped_name);
+ if (ret < 0) {
+ fuse_reply_err(req, -ret);
+ return;
+ }
+ if (mapped_name) {
+ name = mapped_name;
+ }
+ }
+
inode = lo_inode(req, ino);
if (!inode) {
fuse_reply_err(req, EBADF);
+ g_free(mapped_name);
return;
}
@@ -2434,6 +2528,7 @@ out:
}
lo_inode_put(lo, &inode);
+ g_free(mapped_name);
fuse_reply_err(req, saverr);
}
--
2.28.0
On Wed, Oct 14, 2020 at 07:02:06PM +0100, Dr. David Alan Gilbert (git) wrote: > From: "Dr. David Alan Gilbert" <dgilbert@redhat.com> > > Map xattr names originating at the client; from get/set/remove xattr. > > Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com> > --- > tools/virtiofsd/passthrough_ll.c | 101 ++++++++++++++++++++++++++++++- > 1 file changed, 98 insertions(+), 3 deletions(-) Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
On Wed, Oct 14, 2020 at 07:02:06PM +0100, Dr. David Alan Gilbert (git) wrote:
> From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
>
> Map xattr names originating at the client; from get/set/remove xattr.
>
> Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
> ---
> tools/virtiofsd/passthrough_ll.c | 101 ++++++++++++++++++++++++++++++-
> 1 file changed, 98 insertions(+), 3 deletions(-)
>
> diff --git a/tools/virtiofsd/passthrough_ll.c b/tools/virtiofsd/passthrough_ll.c
> index f5a33014f9..57ebe17ed6 100644
> --- a/tools/virtiofsd/passthrough_ll.c
> +++ b/tools/virtiofsd/passthrough_ll.c
> @@ -2183,20 +2183,80 @@ static XattrMapEntry *parse_xattrmap(struct lo_data *lo)
> return res;
> }
>
> -static void lo_getxattr(fuse_req_t req, fuse_ino_t ino, const char *name,
> +/*
> + * For use with getxattr/setxattr/removexattr, where the client
> + * gives us a name and we may need to choose a different one.
> + * Allocates a buffer for the result placing it in *out_name.
> + * If there's no change then *out_name is not set.
> + * Returns 0 on success
> + * Can return -EPERM to indicate we block a given attribute
> + * (in which case out_name is not allocated)
> + * Can return -ENOMEM to indicate out_name couldn't be allocated.
> + */
> +static int xattr_map_client(const struct lo_data *lo, const char *client_name,
> + char **out_name)
> +{
> + const XattrMapEntry *cur_entry;
> +
> + for (cur_entry = lo->xattr_map_list; ; cur_entry++) {
> + if ((cur_entry->flags & XATTR_MAP_FLAG_CLIENT) &&
> + (strstart(client_name, cur_entry->key, NULL))) {
> + if (cur_entry->flags & XATTR_MAP_FLAG_END_BAD) {
> + return -EPERM;
> + }
> + if (cur_entry->flags & XATTR_MAP_FLAG_END_OK) {
> + /* Unmodified name */
> + return 0;
> + }
> + if (cur_entry->flags & XATTR_MAP_FLAG_PREFIX) {
I am wondering why do have "END" substring in BAD and OK flags while
we don't have one in PREFIX flag. IOW, why not simply call these
flags as XATTR_MAP_FLAG_OK and XATTR_MAP_FLAG_BAD respectively.
> + *out_name = g_try_malloc(strlen(client_name) +
> + strlen(cur_entry->prepend) + 1);
Should we check for cur_entry->prepend to be NULL before we try to
allocate out_name. One could say.
"prefix:client:trusted.::". In that case we are not supposed to prefix
anything?
> + if (!*out_name) {
> + return -ENOMEM;
> + }
> + sprintf(*out_name, "%s%s", cur_entry->prepend, client_name);
> + return 0;
> + }
> + }
> + }
> +
> + /* Shouldn't get here - rules should have an END_* */
> + abort();
> +}
Thanks
Vivek
* Vivek Goyal (vgoyal@redhat.com) wrote:
> On Wed, Oct 14, 2020 at 07:02:06PM +0100, Dr. David Alan Gilbert (git) wrote:
> > From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
> >
> > Map xattr names originating at the client; from get/set/remove xattr.
> >
> > Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
> > ---
> > tools/virtiofsd/passthrough_ll.c | 101 ++++++++++++++++++++++++++++++-
> > 1 file changed, 98 insertions(+), 3 deletions(-)
> >
> > diff --git a/tools/virtiofsd/passthrough_ll.c b/tools/virtiofsd/passthrough_ll.c
> > index f5a33014f9..57ebe17ed6 100644
> > --- a/tools/virtiofsd/passthrough_ll.c
> > +++ b/tools/virtiofsd/passthrough_ll.c
> > @@ -2183,20 +2183,80 @@ static XattrMapEntry *parse_xattrmap(struct lo_data *lo)
> > return res;
> > }
> >
> > -static void lo_getxattr(fuse_req_t req, fuse_ino_t ino, const char *name,
> > +/*
> > + * For use with getxattr/setxattr/removexattr, where the client
> > + * gives us a name and we may need to choose a different one.
> > + * Allocates a buffer for the result placing it in *out_name.
> > + * If there's no change then *out_name is not set.
> > + * Returns 0 on success
> > + * Can return -EPERM to indicate we block a given attribute
> > + * (in which case out_name is not allocated)
> > + * Can return -ENOMEM to indicate out_name couldn't be allocated.
> > + */
> > +static int xattr_map_client(const struct lo_data *lo, const char *client_name,
> > + char **out_name)
> > +{
> > + const XattrMapEntry *cur_entry;
> > +
> > + for (cur_entry = lo->xattr_map_list; ; cur_entry++) {
> > + if ((cur_entry->flags & XATTR_MAP_FLAG_CLIENT) &&
> > + (strstart(client_name, cur_entry->key, NULL))) {
> > + if (cur_entry->flags & XATTR_MAP_FLAG_END_BAD) {
> > + return -EPERM;
> > + }
> > + if (cur_entry->flags & XATTR_MAP_FLAG_END_OK) {
> > + /* Unmodified name */
> > + return 0;
> > + }
> > + if (cur_entry->flags & XATTR_MAP_FLAG_PREFIX) {
>
> I am wondering why do have "END" substring in BAD and OK flags while
> we don't have one in PREFIX flag. IOW, why not simply call these
> flags as XATTR_MAP_FLAG_OK and XATTR_MAP_FLAG_BAD respectively.
OK, the END_'s have gone.
> > + *out_name = g_try_malloc(strlen(client_name) +
> > + strlen(cur_entry->prepend) + 1);
>
> Should we check for cur_entry->prepend to be NULL before we try to
> allocate out_name. One could say.
>
> "prefix:client:trusted.::". In that case we are not supposed to prefix
> anything?
We shouldn't need to do the NULL check; cur_entry->prepend should = ""
in that case, not NULL.
Dave
> > + if (!*out_name) {
> > + return -ENOMEM;
> > + }
> > + sprintf(*out_name, "%s%s", cur_entry->prepend, client_name);
> > + return 0;
> > + }
> > + }
> > + }
> > +
> > + /* Shouldn't get here - rules should have an END_* */
> > + abort();
> > +}
>
> Thanks
> Vivek
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
© 2016 - 2026 Red Hat, Inc.