* Vivek Goyal (vgo...@redhat.com) wrote:
> On Wed, Oct 14, 2020 at 07:02:06PM +0100, Dr. David Alan Gilbert (git) wrote:
> > From: "Dr. David Alan Gilbert" <dgilb...@redhat.com>
> >
> > Map xattr names originating at the client; from get/set/remove xattr.
> >
> > Signed-off-by: Dr. David Alan Gilbert <dgilb...@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 / dgilb...@redhat.com / Manchester, UK
