[Qemu-commits] [qemu/qemu] 418638: nbd/server: Implement sparse reads atop structured...

[GitHub]

  Branch: refs/heads/master
  Home:   https://github.com/qemu/qemu
  Commit: 418638d3e448e0ed79d55cc43a26f7a65c22007f
      
https://github.com/qemu/qemu/commit/418638d3e448e0ed79d55cc43a26f7a65c22007f
  Author: Eric Blake <address@hidden>
  Date:   2018-01-08 (Mon, 08 Jan 2018)

  Changed paths:
    M nbd/server.c
    M nbd/trace-events

  Log Message:
  -----------
  nbd/server: Implement sparse reads atop structured reply

The reason that NBD added structured reply in the first place was
to allow for efficient reads of sparse files, by allowing the
reply to include chunks to quickly communicate holes to the client
without sending lots of zeroes over the wire.  Time to implement
this in the server; our client can already read such data.

We can only skip holes insofar as the block layer can query them;
and only if the client is okay with a fragmented request (if a
client requests NBD_CMD_FLAG_DF and the entire read is a hole, we
could technically return a single NBD_REPLY_TYPE_OFFSET_HOLE, but
that's a fringe case not worth catering to here).  Sadly, the
control flow is a bit wonkier than I would have preferred, but
it was minimally invasive to have a split in the action between
a fragmented read (handled directly where we recognize
NBD_CMD_READ with the right conditions, and sending multiple
chunks) vs. a single read (handled at the end of nbd_trip, for
both simple and structured replies, when we know there is only
one thing being read).  Likewise, I didn't make any effort to
optimize the final chunk of a fragmented read to set the
NBD_REPLY_FLAG_DONE, but unconditionally send that as a separate
NBD_REPLY_TYPE_NONE.

Signed-off-by: Eric Blake <address@hidden>
Message-Id: <address@hidden>
Reviewed-by: Vladimir Sementsov-Ogievskiy <address@hidden>


  Commit: e2de3256c373fe32c7a7d9ef2f2093c643bb6656
      
https://github.com/qemu/qemu/commit/e2de3256c373fe32c7a7d9ef2f2093c643bb6656
  Author: Eric Blake <address@hidden>
  Date:   2018-01-08 (Mon, 08 Jan 2018)

  Changed paths:
    M nbd/server.c

  Log Message:
  -----------
  nbd/server: Optimize final chunk of sparse read

If we are careful to handle 0-length read requests correctly,
we can optimize our sparse read to send the NBD_REPLY_FLAG_DONE
bit on our last OFFSET_DATA or OFFSET_HOLE chunk rather than
needing a separate chunk.

Signed-off-by: Eric Blake <address@hidden>
Message-Id: <address@hidden>
Reviewed-by: Vladimir Sementsov-Ogievskiy <address@hidden>


  Commit: c4365735a7d38f4355c6f77e6670d3972315f7c2
      
https://github.com/qemu/qemu/commit/c4365735a7d38f4355c6f77e6670d3972315f7c2
  Author: Murilo Opsfelder Araujo <address@hidden>
  Date:   2018-01-08 (Mon, 08 Jan 2018)

  Changed paths:
    M block/nbd.c

  Log Message:
  -----------
  block/nbd: fix segmentation fault when .desc is not null-terminated

The find_desc_by_name() from util/qemu-option.c relies on the .name not being
NULL to call strcmp(). This check becomes unsafe when the list is not
NULL-terminated, which is the case of nbd_runtime_opts in block/nbd.c, and can
result in segmentation fault when strcmp() tries to access an invalid memory:

    #0 0x00007fff8c75f7d4 in __strcmp_power9 () from /lib64/libc.so.6
    #1 0x00000000102d3ec8 in find_desc_by_name (desc=0x1036d6f0, 
name=0x28e46670 "server.path") at util/qemu-option.c:166
    #2 0x00000000102d93e0 in qemu_opts_absorb_qdict (opts=0x28e47a80, 
qdict=0x28e469a0, errp=0x7fffec247c98) at util/qemu-option.c:1026
    #3 0x000000001012a2e4 in nbd_open (bs=0x28e42290, options=0x28e469a0, 
flags=24578, errp=0x7fffec247d80) at block/nbd.c:406
    #4 0x00000000100144e8 in bdrv_open_driver (bs=0x28e42290, drv=0x1036e070 
<bdrv_nbd_unix>, node_name=0x0, options=0x28e469a0, open_flags=24578, 
errp=0x7fffec247f50) at block.c:1135
    #5 0x0000000010015b04 in bdrv_open_common (bs=0x28e42290, file=0x0, 
options=0x28e469a0, errp=0x7fffec247f50) at block.c:1395

>From gdb, the desc[i].name was not NULL and resulted in strcmp() accessing an
invalid memory:

    >>> p desc[5]
    $8 = {
      name = 0x1037f098 "R27A",
      type = 1561964883,
      help = 0xc0bbb23e <error: Cannot access memory at address 0xc0bbb23e>,
      def_value_str = 0x2 <error: Cannot access memory at address 0x2>
    }
    >>> p desc[6]
    $9 = {
      name = 0x103dac78 <__gcov0.do_qemu_init_bdrv_nbd_init> "\001",
      type = 272101528,
      help = 0x29ec0b754403e31f <error: Cannot access memory at address 
0x29ec0b754403e31f>,
      def_value_str = 0x81f343b9 <error: Cannot access memory at address 
0x81f343b9>
    }

This patch fixes the segmentation fault in strcmp() by adding a NULL element at
the end of nbd_runtime_opts.desc list, which is the common practice to most of
other structs like runtime_opts in block/null.c. Thus, the desc[i].name != NULL
check becomes safe because it will not evaluate to true when .desc list reached
its end.

Reported-by: R. Nageswara Sastry <address@hidden>
Buglink: https://bugs.launchpad.net/qemu/+bug/1727259
Signed-off-by: Murilo Opsfelder Araujo <address@hidden>
Message-Id: <address@hidden>
CC: address@hidden
Fixes: 7ccc44fd7d1dfa62c4d6f3a680df809d6e7068ce
Signed-off-by: Eric Blake <address@hidden>


  Commit: 3cee4db661ab9c0fce7937b3bbfa188a1845f31f
      
https://github.com/qemu/qemu/commit/3cee4db661ab9c0fce7937b3bbfa188a1845f31f
  Author: Peter Maydell <address@hidden>
  Date:   2018-01-09 (Tue, 09 Jan 2018)

  Changed paths:
    M block/nbd.c
    M nbd/server.c
    M nbd/trace-events

  Log Message:
  -----------
  Merge remote-tracking branch 'remotes/ericb/tags/pull-nbd-2018-01-08' into 
staging

nbd patches for 2018-01-08

- Eric Blake: 0/2 Optimize sparse reads over NBD
- Murilo Opsfelder Araujo: block/nbd: fix segmentation fault when .desc is not 
null-terminated

# gpg: Signature made Mon 08 Jan 2018 15:21:19 GMT
# gpg:                using RSA key 0xA7A16B4A2527436A
# gpg: Good signature from "Eric Blake <address@hidden>"
# gpg:                 aka "Eric Blake (Free Software Programmer) 
<address@hidden>"
# gpg:                 aka "[jpeg image of size 6874]"
# Primary key fingerprint: 71C2 CC22 B1C4 6029 27D2  F3AA A7A1 6B4A 2527 436A

* remotes/ericb/tags/pull-nbd-2018-01-08:
  block/nbd: fix segmentation fault when .desc is not null-terminated
  nbd/server: Optimize final chunk of sparse read
  nbd/server: Implement sparse reads atop structured reply

Signed-off-by: Peter Maydell <address@hidden>


Compare: https://github.com/qemu/qemu/compare/ee98a6b089d3...3cee4db661ab