30.03.2019 18:57, Eric Blake wrote:
> The NBD spec suggests that a server should never advertise a size
> inconsistent with its minimum block alignment, as that tail is
> effectively inaccessible to a compliant client obeying those block
> constraints. Since we have a habit of rounding up rather than
> truncating, to avoid losing the last few bytes of user input, and we
> cannot access the tail when the server advertises bogus block sizing,
> abort the connection to alert the server to fix their bug. And
> rejecting such servers matches what we already did for a min_block
> that was not a power of 2 or which was larger than max_block.
>
> Does not impact either qemu (which always sends properly aligned
> sizes) or nbdkit (which does not send minimum block requirements yet);
> so this is mostly aimed at new NBD server implementations, and ensures
> that the rest of our code can assume the size is aligned.
>
> Signed-off-by: Eric Blake <ebl...@redhat.com>
> Message-Id: <20190329163407.26919-1-ebl...@redhat.com>
> ---
>
> This is the rewrite of the original 7/6 that I posted, retitled and
> hoisted earlier in the series (since Vladimir was right that 4/6
> depends on it). I've added patches 1-3 to my NBD queue for a pull
> request on Monday as they now have R-b; I'd love to also have enough
> reviews for 3.5, 4, 5, 6, and also another thread on fixing 'qemu-img
> info' output, to include those as well.
>
> nbd/client.c | 8 ++++++++
> 1 file changed, 8 insertions(+)
>
> diff --git a/nbd/client.c b/nbd/client.c
> index de7da48246b..9f5eb79930c 100644
> --- a/nbd/client.c
> +++ b/nbd/client.c
> @@ -426,6 +426,14 @@ static int nbd_opt_info_or_go(QIOChannel *ioc, uint32_t
> opt,
> nbd_send_opt_abort(ioc);
> return -1;
> }
> + if (info->min_block &&
> + !QEMU_IS_ALIGNED(info->size, info->min_block)) {
> + error_setg(errp, "server size %" PRIu64 "is not multiple of "
s/server/export/ I think
> + "minimum block size %" PRIu32, info->size,
> + info->min_block);
> + nbd_send_opt_abort(ioc);
> + return -1;
> + }
> trace_nbd_receive_negotiate_size_flags(info->size, info->flags);
> break;
>
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsement...@virtuozzo.com>
--
Best regards,
Vladimir
