linux-kernel - Re: [PATCH 2/2] nbd: add support for nbd as root device

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <3a15a92f-16d6-cca3-8842-b0866579a95a@redhat.com>
Date:   Thu, 13 Jun 2019 07:56:38 -0500
From:   Eric Blake <eblake@...hat.com>
To:     roman.stratiienko@...ballogic.com, linux-kernel@...r.kernel.org,
        josef@...icpanda.com, nbd@...er.debian.org,
        A.Bulyshchenko@...ballogic.com, linux-block@...r.kernel.org,
        axboe@...nel.dkn.org
Subject: Re: [PATCH 2/2] nbd: add support for nbd as root device

On 6/12/19 11:31 AM, roman.stratiienko@...ballogic.com wrote:
> From: Roman Stratiienko <roman.stratiienko@...ballogic.com>
> 
> Adding support to nbd to use it as a root device. This code essentially
> provides a minimal nbd-client implementation within the kernel. It opens
> a socket and makes the negotiation with the server. Afterwards it passes
> the socket to the normal nbd-code to handle the connection.
> 
> The arguments for the server are passed via kernel command line.
> The kernel command line has the format
> 'nbdroot=[<SERVER_IP>:]<SERVER_PORT>/<EXPORT_NAME>'.
> SERVER_IP is optional. If it is not available it will use the
> root_server_addr transmitted through DHCP.
> 
> Based on those arguments, the connection to the server is established
> and is connected to the nbd0 device. The rootdevice therefore is
> root=/dev/nbd0.
> 
> Patch was initialy posted by Markus Pargmann <mpa@...gutronix.de>
> and can be found at https://lore.kernel.org/patchwork/patch/532556/
> 
> Change-Id: I78f7313918bf31b9dc01a74a42f0f068bede312c
> Signed-off-by: Roman Stratiienko <roman.stratiienko@...ballogic.com>
> Reviewed-by: Aleksandr Bulyshchenko <A.Bulyshchenko@...ballogic.com>

> +static int nbd_connection_negotiate(struct socket *sock, char *export_name,
> +				    size_t *rsize, u16 *nflags)
> +{

> +
> +	ret = sock_xmit_buf(sock, 0, &flags, sizeof(flags));
> +	if (ret < 0)
> +		return ret;
> +
> +	*nflags = ntohs(flags);
> +
> +	client_flags = 0;

Why not reply with NBD_FLAG_C_FIXED_NEWSTYLE? Which in turn would be
necessary...

> +
> +	ret = sock_xmit_buf(sock, 1, &client_flags, sizeof(client_flags));
> +	if (ret < 0)
> +		return ret;
> +
> +	magic = cpu_to_be64(nbd_opts_magic);
> +	ret = sock_xmit_buf(sock, 1, &magic, sizeof(magic));
> +	if (ret < 0)
> +		return ret;
> +
> +	opt = htonl(NBD_OPT_EXPORT_NAME);

Why not use NBD_OPT_GO?  That's a better interface (better error
recovery, and some servers can even use it to advertise preferred block
sizing - particularly if a server insists on a 4k minimum block instead
of 512).

...using NBD_OPT_GO would require checking that the server advertised
FIXED_NEWSTYLE as well as replying that we intend to rely on it.

Otherwise the idea looks interesting to me.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org



Download attachment "signature.asc" of type "application/pgp-signature" (489 bytes)