diff mbox

[4/4] hw/9pfs: fix P9_STATS_GEN handling

Message ID 1390921707-15109-4-git-send-email-kirill.shutemov@linux.intel.com
State New
Headers show

Commit Message

Kirill A. Shutemov Jan. 28, 2014, 3:08 p.m. UTC
Currently we fail getattr request altogether if we can't read
P9_STATS_GEN for some reason. It breaks valid use cases:

E.g let's assume we have non-readable directory with execution bit set
on host and we export it to client over 9p On host we can chdir into
directory, but not open directory on read and list content.

But if client will try to call getattr (as part of chdir(2)) for the
directory it will fail with -EACCES. It happens because we try to open
the directory on read to call ioctl(FS_IOC_GETVERSION), it fails and we
return the error code to client.

It's excessive. The solution is to make P9_STATS_GEN failure non-fatal
for getattr request. Just don't set P9_STATS_GEN flag in result mask on
failure.

Signed-off-by: Kirill A. Shutemov <kirill.shutemov@linux.intel.com>
---
 hw/9pfs/cofile.c    |  4 ----
 hw/9pfs/virtio-9p.c | 12 ++++++++++--
 2 files changed, 10 insertions(+), 6 deletions(-)

Comments

Aneesh Kumar K.V Feb. 2, 2014, 4:27 p.m. UTC | #1
"Kirill A. Shutemov" <kirill.shutemov@linux.intel.com> writes:

> Currently we fail getattr request altogether if we can't read
> P9_STATS_GEN for some reason. It breaks valid use cases:
>
> E.g let's assume we have non-readable directory with execution bit set
> on host and we export it to client over 9p On host we can chdir into
> directory, but not open directory on read and list content.
>
> But if client will try to call getattr (as part of chdir(2)) for the
> directory it will fail with -EACCES. It happens because we try to open
> the directory on read to call ioctl(FS_IOC_GETVERSION), it fails and we
> return the error code to client.
>
> It's excessive. The solution is to make P9_STATS_GEN failure non-fatal
> for getattr request. Just don't set P9_STATS_GEN flag in result mask on
> failure.
>
> Signed-off-by: Kirill A. Shutemov <kirill.shutemov@linux.intel.com>


Reviewed-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>

> ---
>  hw/9pfs/cofile.c    |  4 ----
>  hw/9pfs/virtio-9p.c | 12 ++++++++++--
>  2 files changed, 10 insertions(+), 6 deletions(-)
>
> diff --git a/hw/9pfs/cofile.c b/hw/9pfs/cofile.c
> index 194c1306c665..2efebf35710f 100644
> --- a/hw/9pfs/cofile.c
> +++ b/hw/9pfs/cofile.c
> @@ -38,10 +38,6 @@ int v9fs_co_st_gen(V9fsPDU *pdu, V9fsPath *path, mode_t st_mode,
>              });
>          v9fs_path_unlock(s);
>      }
> -    /* The ioctl may not be supported depending on the path */
> -    if (err == -ENOTTY) {
> -        err = 0;
> -    }
>      return err;
>  }
>
> diff --git a/hw/9pfs/virtio-9p.c b/hw/9pfs/virtio-9p.c
> index 8cbb8ae32a03..83e4e9398347 100644
> --- a/hw/9pfs/virtio-9p.c
> +++ b/hw/9pfs/virtio-9p.c
> @@ -1080,10 +1080,18 @@ static void v9fs_getattr(void *opaque)
>      /*  fill st_gen if requested and supported by underlying fs */
>      if (request_mask & P9_STATS_GEN) {
>          retval = v9fs_co_st_gen(pdu, &fidp->path, stbuf.st_mode, &v9stat_dotl);
> -        if (retval < 0) {
> +        switch (retval) {
> +        case 0:
> +            /* we have valid st_gen: update result mask */
> +            v9stat_dotl.st_result_mask |= P9_STATS_GEN;
> +            break;
> +        case -EINTR:
> +            /* request cancelled, e.g. by Tflush */
>              goto out;
> +        default:
> +            /* failed to get st_gen: not fatal, ignore */
> +            break;
>          }
> -        v9stat_dotl.st_result_mask |= P9_STATS_GEN;
>      }
>      retval = pdu_marshal(pdu, offset, "A", &v9stat_dotl);
>      if (retval < 0) {
> -- 
> 1.8.5.2
diff mbox

Patch

diff --git a/hw/9pfs/cofile.c b/hw/9pfs/cofile.c
index 194c1306c665..2efebf35710f 100644
--- a/hw/9pfs/cofile.c
+++ b/hw/9pfs/cofile.c
@@ -38,10 +38,6 @@  int v9fs_co_st_gen(V9fsPDU *pdu, V9fsPath *path, mode_t st_mode,
             });
         v9fs_path_unlock(s);
     }
-    /* The ioctl may not be supported depending on the path */
-    if (err == -ENOTTY) {
-        err = 0;
-    }
     return err;
 }
 
diff --git a/hw/9pfs/virtio-9p.c b/hw/9pfs/virtio-9p.c
index 8cbb8ae32a03..83e4e9398347 100644
--- a/hw/9pfs/virtio-9p.c
+++ b/hw/9pfs/virtio-9p.c
@@ -1080,10 +1080,18 @@  static void v9fs_getattr(void *opaque)
     /*  fill st_gen if requested and supported by underlying fs */
     if (request_mask & P9_STATS_GEN) {
         retval = v9fs_co_st_gen(pdu, &fidp->path, stbuf.st_mode, &v9stat_dotl);
-        if (retval < 0) {
+        switch (retval) {
+        case 0:
+            /* we have valid st_gen: update result mask */
+            v9stat_dotl.st_result_mask |= P9_STATS_GEN;
+            break;
+        case -EINTR:
+            /* request cancelled, e.g. by Tflush */
             goto out;
+        default:
+            /* failed to get st_gen: not fatal, ignore */
+            break;
         }
-        v9stat_dotl.st_result_mask |= P9_STATS_GEN;
     }
     retval = pdu_marshal(pdu, offset, "A", &v9stat_dotl);
     if (retval < 0) {