[Qemu-devel] [PATCH v2] v9fs_walk: As per 9p2000 RFC, MAXWELEM >= nwnames >= 0.

[Qemu-devel] [PATCH v2] v9fs_walk: As per 9p2000 RFC, MAXWELEM >= nwnames >= 0.

[Harsh Prateek Bora]

The nwnames field in TWALK message is assumed to be >=0 and <= MAXWELEM
which is defined as macro P9_MAXWELEM (16) in virtio-9p.h as per 9p2000 RFC.
Appropriate changes are required in V9fsWalkState and v9fs_walk.

v2:
- Added check in v9fs_walk_complete as well.

Signed-off-by: Harsh Prateek Bora <harsh@linux.vnet.ibm.com>
---
 hw/9pfs/virtio-9p.c |    7 +++++--
 hw/9pfs/virtio-9p.h |    2 +-
 2 files changed, 6 insertions(+), 3 deletions(-)

diff --git a/hw/9pfs/virtio-9p.c b/hw/9pfs/virtio-9p.c
index 9b44bd0..dde51bc 100644
--- a/hw/9pfs/virtio-9p.c
+++ b/hw/9pfs/virtio-9p.c
@@ -1708,7 +1708,7 @@ static void v9fs_walk_complete(V9fsState *s, V9fsWalkState *vs, int err)
 {
     complete_pdu(s, vs->pdu, err);
 
-    if (vs->nwnames) {
+    if (vs->nwnames <= P9_MAXWELEM) {
         for (vs->name_idx = 0; vs->name_idx < vs->nwnames; vs->name_idx++) {
             v9fs_string_free(&vs->wnames[vs->name_idx]);
         }
@@ -1805,7 +1805,7 @@ static void v9fs_walk(V9fsState *s, V9fsPDU *pdu)
     vs->offset += pdu_unmarshal(vs->pdu, vs->offset, "ddw", &fid,
                                             &newfid, &vs->nwnames);
 
-    if (vs->nwnames) {
+    if (vs->nwnames <= P9_MAXWELEM) {
         vs->wnames = qemu_mallocz(sizeof(vs->wnames[0]) * vs->nwnames);
 
         vs->qids = qemu_mallocz(sizeof(vs->qids[0]) * vs->nwnames);
@@ -1814,6 +1814,9 @@ static void v9fs_walk(V9fsState *s, V9fsPDU *pdu)
             vs->offset += pdu_unmarshal(vs->pdu, vs->offset, "s",
                                             &vs->wnames[i]);
         }
+    } else {
+        err = -EINVAL;
+        goto out;
     }
 
     vs->fidp = lookup_fid(s, fid);
diff --git a/hw/9pfs/virtio-9p.h b/hw/9pfs/virtio-9p.h
index e7d2326..119b12c 100644
--- a/hw/9pfs/virtio-9p.h
+++ b/hw/9pfs/virtio-9p.h
@@ -307,7 +307,7 @@ typedef struct V9fsStatStateDotl {
 typedef struct V9fsWalkState {
     V9fsPDU *pdu;
     size_t offset;
-    int16_t nwnames;
+    uint16_t nwnames;
     int name_idx;
     V9fsQID *qids;
     V9fsFidState *fidp;
-- 
1.7.1.1

Re: [Qemu-devel] [PATCH v2] v9fs_walk: As per 9p2000 RFC, MAXWELEM >= nwnames >= 0.

[Harsh Bora]

Hi,
Review comments / ACK please ?

regards,
Harsh

On 03/22/2011 11:21 AM, Harsh Prateek Bora wrote:
> The nwnames field in TWALK message is assumed to be>=0 and<= MAXWELEM
> which is defined as macro P9_MAXWELEM (16) in virtio-9p.h as per 9p2000 RFC.
> Appropriate changes are required in V9fsWalkState and v9fs_walk.
>
> v2:
> - Added check in v9fs_walk_complete as well.
>
> Signed-off-by: Harsh Prateek Bora<harsh@linux.vnet.ibm.com>
> ---
>   hw/9pfs/virtio-9p.c |    7 +++++--
>   hw/9pfs/virtio-9p.h |    2 +-
>   2 files changed, 6 insertions(+), 3 deletions(-)
>
> diff --git a/hw/9pfs/virtio-9p.c b/hw/9pfs/virtio-9p.c
> index 9b44bd0..dde51bc 100644
> --- a/hw/9pfs/virtio-9p.c
> +++ b/hw/9pfs/virtio-9p.c
> @@ -1708,7 +1708,7 @@ static void v9fs_walk_complete(V9fsState *s, V9fsWalkState *vs, int err)
>   {
>       complete_pdu(s, vs->pdu, err);
>
> -    if (vs->nwnames) {
> +    if (vs->nwnames<= P9_MAXWELEM) {
>           for (vs->name_idx = 0; vs->name_idx<  vs->nwnames; vs->name_idx++) {
>               v9fs_string_free(&vs->wnames[vs->name_idx]);
>           }
> @@ -1805,7 +1805,7 @@ static void v9fs_walk(V9fsState *s, V9fsPDU *pdu)
>       vs->offset += pdu_unmarshal(vs->pdu, vs->offset, "ddw",&fid,
>                                               &newfid,&vs->nwnames);
>
> -    if (vs->nwnames) {
> +    if (vs->nwnames<= P9_MAXWELEM) {
>           vs->wnames = qemu_mallocz(sizeof(vs->wnames[0]) * vs->nwnames);
>
>           vs->qids = qemu_mallocz(sizeof(vs->qids[0]) * vs->nwnames);
> @@ -1814,6 +1814,9 @@ static void v9fs_walk(V9fsState *s, V9fsPDU *pdu)
>               vs->offset += pdu_unmarshal(vs->pdu, vs->offset, "s",
>                                               &vs->wnames[i]);
>           }
> +    } else {
> +        err = -EINVAL;
> +        goto out;
>       }
>
>       vs->fidp = lookup_fid(s, fid);
> diff --git a/hw/9pfs/virtio-9p.h b/hw/9pfs/virtio-9p.h
> index e7d2326..119b12c 100644
> --- a/hw/9pfs/virtio-9p.h
> +++ b/hw/9pfs/virtio-9p.h
> @@ -307,7 +307,7 @@ typedef struct V9fsStatStateDotl {
>   typedef struct V9fsWalkState {
>       V9fsPDU *pdu;
>       size_t offset;
> -    int16_t nwnames;
> +    uint16_t nwnames;
>       int name_idx;
>       V9fsQID *qids;
>       V9fsFidState *fidp;

[Qemu-devel] Re: [PATCH v2] v9fs_walk: As per 9p2000 RFC, MAXWELEM >= nwnames >= 0.

[Stefan Hajnoczi]

On Tue, Mar 22, 2011 at 5:51 AM, Harsh Prateek Bora
<harsh@linux.vnet.ibm.com> wrote:
> The nwnames field in TWALK message is assumed to be >=0 and <= MAXWELEM
> which is defined as macro P9_MAXWELEM (16) in virtio-9p.h as per 9p2000 RFC.
> Appropriate changes are required in V9fsWalkState and v9fs_walk.
>
> v2:
> - Added check in v9fs_walk_complete as well.
>
> Signed-off-by: Harsh Prateek Bora <harsh@linux.vnet.ibm.com>
> ---
>  hw/9pfs/virtio-9p.c |    7 +++++--
>  hw/9pfs/virtio-9p.h |    2 +-
>  2 files changed, 6 insertions(+), 3 deletions(-)

Reviewed-by: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com>