qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [Qemu-trivial] [PATCH for 2.10 v2 18/20] 9pfs: avoid si


From: Philippe Mathieu-Daudé
Subject: Re: [Qemu-devel] [Qemu-trivial] [PATCH for 2.10 v2 18/20] 9pfs: avoid sign conversion error simplifying the code
Date: Thu, 27 Jul 2017 16:18:07 -0300
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.2.1

On 07/27/2017 08:40 AM, Greg Kurz wrote:
On Wed, 26 Jul 2017 23:42:22 -0300
Philippe Mathieu-Daudé <address@hidden> wrote:
Reviewed-by: Greg Kurz <address@hidden>

Now, I'm not sure this can be merged during hard freeze since it is
more code cleanup than actual bug fixing...

Hmm the commit message is probably not enough.
The problem is this code can send broken packets, see inlined:


  hw/9pfs/9p.c | 6 ++----
  1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/hw/9pfs/9p.c b/hw/9pfs/9p.c
index 333dbb6f8e..0a37c8bd13 100644
--- a/hw/9pfs/9p.c
+++ b/hw/9pfs/9p.c
@@ -945,7 +945,6 @@ static void coroutine_fn v9fs_version(void *opaque)
      v9fs_string_init(&version);
      err = pdu_unmarshal(pdu, offset, "ds", &s->msize, &version);
      if (err < 0) {

if err == -1

-        offset = err;

here this sets offset = (size_t)(-1) = SIZE_MAX

          goto out;
      }
      trace_v9fs_version(pdu->tag, pdu->id, s->msize, version.data);
@@ -962,13 +961,12 @@ static void coroutine_fn v9fs_version(void *opaque)
err = pdu_marshal(pdu, offset, "ds", s->msize, &version);
      if (err < 0) {
-        offset = err;
          goto out;
      }
-    offset += err;

here offset += SIZE_MAX which wraps, so this equivs to offset -= 1

+    err += offset;
      trace_v9fs_version_return(pdu->tag, pdu->id, s->msize, version.data);
  out:
-    pdu_complete(pdu, offset);

now we have offset = 7 - 1 = 6, since 6 > 0 pdu_complete() does not marshal the error code but 6 bytes of crap from pdu?

Maybe I missed something.

+    pdu_complete(pdu, err);
      v9fs_string_free(&version);
  }

Regards,

Phil.



reply via email to

[Prev in Thread] Current Thread [Next in Thread]