[PATCH 1/4] virtiofsd: Remove fuse_req_getgroups

Dr. David Alan Gilbert (git) posted 4 patches 6 years ago
Maintainers: Stefan Hajnoczi <stefanha@redhat.com>, "Dr. David Alan Gilbert" <dgilbert@redhat.com>
[PATCH 1/4] virtiofsd: Remove fuse_req_getgroups
Posted by Dr. David Alan Gilbert (git) 6 years ago
From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>

Remove fuse_req_getgroups that's unused in virtiofsd; it came in
from libfuse but we don't actually use it.  It was called from
fuse_getgroups which we previously removed (but had left it's header
in).

Coverity had complained about null termination in it, but removing
it is the easiest answer.

Fixes: Coverity CID: 1413117 (String not null terminated)
Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
---
 tools/virtiofsd/fuse.h          | 20 ---------
 tools/virtiofsd/fuse_lowlevel.c | 77 ---------------------------------
 tools/virtiofsd/fuse_lowlevel.h | 21 ---------
 3 files changed, 118 deletions(-)

diff --git a/tools/virtiofsd/fuse.h b/tools/virtiofsd/fuse.h
index 7a4c713559..aba13fef2d 100644
--- a/tools/virtiofsd/fuse.h
+++ b/tools/virtiofsd/fuse.h
@@ -1006,26 +1006,6 @@ void fuse_exit(struct fuse *f);
  */
 struct fuse_context *fuse_get_context(void);
 
-/**
- * Get the current supplementary group IDs for the current request
- *
- * Similar to the getgroups(2) system call, except the return value is
- * always the total number of group IDs, even if it is larger than the
- * specified size.
- *
- * The current fuse kernel module in linux (as of 2.6.30) doesn't pass
- * the group list to userspace, hence this function needs to parse
- * "/proc/$TID/task/$TID/status" to get the group IDs.
- *
- * This feature may not be supported on all operating systems.  In
- * such a case this function will return -ENOSYS.
- *
- * @param size size of given array
- * @param list array of group IDs to be filled in
- * @return the total number of supplementary group IDs or -errno on failure
- */
-int fuse_getgroups(int size, gid_t list[]);
-
 /**
  * Check if the current request has already been interrupted
  *
diff --git a/tools/virtiofsd/fuse_lowlevel.c b/tools/virtiofsd/fuse_lowlevel.c
index de2e2e0c65..01c418aade 100644
--- a/tools/virtiofsd/fuse_lowlevel.c
+++ b/tools/virtiofsd/fuse_lowlevel.c
@@ -2667,83 +2667,6 @@ int fuse_lowlevel_is_virtio(struct fuse_session *se)
     return !!se->virtio_dev;
 }
 
-#ifdef linux
-int fuse_req_getgroups(fuse_req_t req, int size, gid_t list[])
-{
-    char *buf;
-    size_t bufsize = 1024;
-    char path[128];
-    int ret;
-    int fd;
-    unsigned long pid = req->ctx.pid;
-    char *s;
-
-    sprintf(path, "/proc/%lu/task/%lu/status", pid, pid);
-
-retry:
-    buf = malloc(bufsize);
-    if (buf == NULL) {
-        return -ENOMEM;
-    }
-
-    ret = -EIO;
-    fd = open(path, O_RDONLY);
-    if (fd == -1) {
-        goto out_free;
-    }
-
-    ret = read(fd, buf, bufsize);
-    close(fd);
-    if (ret < 0) {
-        ret = -EIO;
-        goto out_free;
-    }
-
-    if ((size_t)ret == bufsize) {
-        free(buf);
-        bufsize *= 4;
-        goto retry;
-    }
-
-    ret = -EIO;
-    s = strstr(buf, "\nGroups:");
-    if (s == NULL) {
-        goto out_free;
-    }
-
-    s += 8;
-    ret = 0;
-    while (1) {
-        char *end;
-        unsigned long val = strtoul(s, &end, 0);
-        if (end == s) {
-            break;
-        }
-
-        s = end;
-        if (ret < size) {
-            list[ret] = val;
-        }
-        ret++;
-    }
-
-out_free:
-    free(buf);
-    return ret;
-}
-#else /* linux */
-/*
- * This is currently not implemented on other than Linux...
- */
-int fuse_req_getgroups(fuse_req_t req, int size, gid_t list[])
-{
-    (void)req;
-    (void)size;
-    (void)list;
-    return -ENOSYS;
-}
-#endif
-
 void fuse_session_exit(struct fuse_session *se)
 {
     se->exited = 1;
diff --git a/tools/virtiofsd/fuse_lowlevel.h b/tools/virtiofsd/fuse_lowlevel.h
index 138041e5f1..8f6d705b5c 100644
--- a/tools/virtiofsd/fuse_lowlevel.h
+++ b/tools/virtiofsd/fuse_lowlevel.h
@@ -1704,27 +1704,6 @@ void *fuse_req_userdata(fuse_req_t req);
  */
 const struct fuse_ctx *fuse_req_ctx(fuse_req_t req);
 
-/**
- * Get the current supplementary group IDs for the specified request
- *
- * Similar to the getgroups(2) system call, except the return value is
- * always the total number of group IDs, even if it is larger than the
- * specified size.
- *
- * The current fuse kernel module in linux (as of 2.6.30) doesn't pass
- * the group list to userspace, hence this function needs to parse
- * "/proc/$TID/task/$TID/status" to get the group IDs.
- *
- * This feature may not be supported on all operating systems.  In
- * such a case this function will return -ENOSYS.
- *
- * @param req request handle
- * @param size size of given array
- * @param list array of group IDs to be filled in
- * @return the total number of supplementary group IDs or -errno on failure
- */
-int fuse_req_getgroups(fuse_req_t req, int size, gid_t list[]);
-
 /**
  * Callback function for an interrupt
  *
-- 
2.24.1


Re: [PATCH 1/4] virtiofsd: Remove fuse_req_getgroups
Posted by Philippe Mathieu-Daudé 6 years ago
On 2/4/20 12:04 PM, Dr. David Alan Gilbert (git) wrote:
> From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
> 
> Remove fuse_req_getgroups that's unused in virtiofsd; it came in
> from libfuse but we don't actually use it.  It was called from
> fuse_getgroups which we previously removed (but had left it's header
> in).
> 
> Coverity had complained about null termination in it, but removing
> it is the easiest answer.

:)

Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>

> 
> Fixes: Coverity CID: 1413117 (String not null terminated)
> Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
> ---
>   tools/virtiofsd/fuse.h          | 20 ---------
>   tools/virtiofsd/fuse_lowlevel.c | 77 ---------------------------------
>   tools/virtiofsd/fuse_lowlevel.h | 21 ---------
>   3 files changed, 118 deletions(-)
> 
> diff --git a/tools/virtiofsd/fuse.h b/tools/virtiofsd/fuse.h
> index 7a4c713559..aba13fef2d 100644
> --- a/tools/virtiofsd/fuse.h
> +++ b/tools/virtiofsd/fuse.h
> @@ -1006,26 +1006,6 @@ void fuse_exit(struct fuse *f);
>    */
>   struct fuse_context *fuse_get_context(void);
>   
> -/**
> - * Get the current supplementary group IDs for the current request
> - *
> - * Similar to the getgroups(2) system call, except the return value is
> - * always the total number of group IDs, even if it is larger than the
> - * specified size.
> - *
> - * The current fuse kernel module in linux (as of 2.6.30) doesn't pass
> - * the group list to userspace, hence this function needs to parse
> - * "/proc/$TID/task/$TID/status" to get the group IDs.
> - *
> - * This feature may not be supported on all operating systems.  In
> - * such a case this function will return -ENOSYS.
> - *
> - * @param size size of given array
> - * @param list array of group IDs to be filled in
> - * @return the total number of supplementary group IDs or -errno on failure
> - */
> -int fuse_getgroups(int size, gid_t list[]);
> -
>   /**
>    * Check if the current request has already been interrupted
>    *
> diff --git a/tools/virtiofsd/fuse_lowlevel.c b/tools/virtiofsd/fuse_lowlevel.c
> index de2e2e0c65..01c418aade 100644
> --- a/tools/virtiofsd/fuse_lowlevel.c
> +++ b/tools/virtiofsd/fuse_lowlevel.c
> @@ -2667,83 +2667,6 @@ int fuse_lowlevel_is_virtio(struct fuse_session *se)
>       return !!se->virtio_dev;
>   }
>   
> -#ifdef linux
> -int fuse_req_getgroups(fuse_req_t req, int size, gid_t list[])
> -{
> -    char *buf;
> -    size_t bufsize = 1024;
> -    char path[128];
> -    int ret;
> -    int fd;
> -    unsigned long pid = req->ctx.pid;
> -    char *s;
> -
> -    sprintf(path, "/proc/%lu/task/%lu/status", pid, pid);
> -
> -retry:
> -    buf = malloc(bufsize);
> -    if (buf == NULL) {
> -        return -ENOMEM;
> -    }
> -
> -    ret = -EIO;
> -    fd = open(path, O_RDONLY);
> -    if (fd == -1) {
> -        goto out_free;
> -    }
> -
> -    ret = read(fd, buf, bufsize);
> -    close(fd);
> -    if (ret < 0) {
> -        ret = -EIO;
> -        goto out_free;
> -    }
> -
> -    if ((size_t)ret == bufsize) {
> -        free(buf);
> -        bufsize *= 4;
> -        goto retry;
> -    }
> -
> -    ret = -EIO;
> -    s = strstr(buf, "\nGroups:");
> -    if (s == NULL) {
> -        goto out_free;
> -    }
> -
> -    s += 8;
> -    ret = 0;
> -    while (1) {
> -        char *end;
> -        unsigned long val = strtoul(s, &end, 0);
> -        if (end == s) {
> -            break;
> -        }
> -
> -        s = end;
> -        if (ret < size) {
> -            list[ret] = val;
> -        }
> -        ret++;
> -    }
> -
> -out_free:
> -    free(buf);
> -    return ret;
> -}
> -#else /* linux */
> -/*
> - * This is currently not implemented on other than Linux...
> - */
> -int fuse_req_getgroups(fuse_req_t req, int size, gid_t list[])
> -{
> -    (void)req;
> -    (void)size;
> -    (void)list;
> -    return -ENOSYS;
> -}
> -#endif
> -
>   void fuse_session_exit(struct fuse_session *se)
>   {
>       se->exited = 1;
> diff --git a/tools/virtiofsd/fuse_lowlevel.h b/tools/virtiofsd/fuse_lowlevel.h
> index 138041e5f1..8f6d705b5c 100644
> --- a/tools/virtiofsd/fuse_lowlevel.h
> +++ b/tools/virtiofsd/fuse_lowlevel.h
> @@ -1704,27 +1704,6 @@ void *fuse_req_userdata(fuse_req_t req);
>    */
>   const struct fuse_ctx *fuse_req_ctx(fuse_req_t req);
>   
> -/**
> - * Get the current supplementary group IDs for the specified request
> - *
> - * Similar to the getgroups(2) system call, except the return value is
> - * always the total number of group IDs, even if it is larger than the
> - * specified size.
> - *
> - * The current fuse kernel module in linux (as of 2.6.30) doesn't pass
> - * the group list to userspace, hence this function needs to parse
> - * "/proc/$TID/task/$TID/status" to get the group IDs.
> - *
> - * This feature may not be supported on all operating systems.  In
> - * such a case this function will return -ENOSYS.
> - *
> - * @param req request handle
> - * @param size size of given array
> - * @param list array of group IDs to be filled in
> - * @return the total number of supplementary group IDs or -errno on failure
> - */
> -int fuse_req_getgroups(fuse_req_t req, int size, gid_t list[]);
> -
>   /**
>    * Callback function for an interrupt
>    *
>