[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-trivial] [PATCH for 2.10 v2 18/20] 9pfs: avoid sign conversion
From: |
Greg Kurz |
Subject: |
Re: [Qemu-trivial] [PATCH for 2.10 v2 18/20] 9pfs: avoid sign conversion error simplifying the code |
Date: |
Fri, 28 Jul 2017 09:49:33 +0200 |
On Thu, 27 Jul 2017 16:18:07 -0300
Philippe Mathieu-Daudé <address@hidden> wrote:
> 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;
and here we jump directly to the out label, so...
> >> }
> >> 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
>
... this cannot happen.
> >> + 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.
I've pushed this to my 9p-next branch to be merged in 2.11.
Cheers,
--
Greg
pgp8LaA_562rN.pgp
Description: OpenPGP digital signature
- Re: [Qemu-trivial] [PATCH for 2.10 v2 12/20] syscall: fix dereference of undefined pointer, (continued)
- [Qemu-trivial] [PATCH for 2.10 v2 13/20] syscall: fix use of uninitialized values, Philippe Mathieu-Daudé, 2017/07/26
- [Qemu-trivial] [PATCH for 2.10 v2 14/20] syscall: check inotify() and eventfd() return value, Philippe Mathieu-Daudé, 2017/07/26
- [Qemu-trivial] [PATCH for 2.10 v2 15/20] thunk: assert nb_fields is valid, Philippe Mathieu-Daudé, 2017/07/26
- [Qemu-trivial] [PATCH for 2.10 v2 17/20] bt-sdp: fix memory leak in sdp_service_record_build(), Philippe Mathieu-Daudé, 2017/07/26
- [Qemu-trivial] [PATCH for 2.10 v2 18/20] 9pfs: avoid sign conversion error simplifying the code, Philippe Mathieu-Daudé, 2017/07/26
- [Qemu-trivial] [PATCH for 2.10 v2 19/20] spapr_vio: fix overflow of qdevs in spapr_dt_vdevice(), Philippe Mathieu-Daudé, 2017/07/26
- [Qemu-trivial] [PATCH for 2.10 v2 20/20] i2c/exynos4210: fix write to I2CADD register, bit 0 is not mapped, Philippe Mathieu-Daudé, 2017/07/26
- Re: [Qemu-trivial] [PATCH for 2.10 v2 00/20] fix bugs reported by Clang Static Analyzer, Michael Tokarev, 2017/07/28