[PATCH] ioctl: implement NBD_* ioctl decoding

[Eugene Syromiatnikov]

On Fri, Sep 21, 2018 at 04:42:50PM +0200, Elvira Khabirova wrote:
> diff --git a/nbd_ioctl.c b/nbd_ioctl.c
> new file mode 100644

It is probably worth including <linux/nbd.h> if it's present, in order
to check that there were no mistakes in constant definitions (and catch
potential UAPI changes early).

> +int
> +nbd_ioctl(struct tcb *const tcp, const unsigned int code,
> +	  const kernel_ulong_t arg)
> +{
> +	if (!verbose(tcp))
> +		return RVAL_DECODED;

I think that it's barely needed here, as decoding of the system call
doesn't require additional tracee's memory accesses (and all commands,
except NBD_SET_FLAGS, have output about the size of a pointer).


> diff --git a/tests/ioctl_nbd.c b/tests/ioctl_nbd.c
> new file mode 100644
> index 00000000..c42a3bcc
> --- /dev/null
> +++ b/tests/ioctl_nbd.c

> +#ifdef HAVE_LINUX_NBD_H

I do not see any entities that are used exclusively from <linux/nbd.h>,
all constants are backed by xlats.

> +int
> +main(void)
> +{
> +	int64_t ubeef = 0xcafef00ddeadbeefULL;

Why not declare it as unsigned long from the beginning? It would allow
avoiding casts below, which also extend beyond 80 columns.

It is also probably worth adding const and static qualifiers.

> +	int sock = socket(AF_UNIX, SOCK_STREAM, 0);

What if socket() returns -1?

> +	ioctl(-1, NBD_SET_FLAGS, 0);
> +	printf("ioctl(-1, NBD_SET_FLAGS, 0) = -1 EBADF (%m)\n");
> +	ioctl(-1, NBD_SET_FLAGS, NBD_FLAG_HAS_FLAGS);
> +	printf("ioctl(-1, NBD_SET_FLAGS, NBD_FLAG_HAS_FLAGS) = -1 EBADF (%m)\n");
> +	ioctl(-1, NBD_SET_FLAGS, NBD_FLAG_READ_ONLY);
> +	printf("ioctl(-1, NBD_SET_FLAGS, NBD_FLAG_READ_ONLY) = -1 EBADF (%m)\n");
> +	ioctl(-1, NBD_SET_FLAGS, NBD_FLAG_SEND_FLUSH);
> +	printf("ioctl(-1, NBD_SET_FLAGS, NBD_FLAG_SEND_FLUSH) = -1 EBADF (%m)\n");
> +	ioctl(-1, NBD_SET_FLAGS, NBD_FLAG_SEND_FUA);
> +	printf("ioctl(-1, NBD_SET_FLAGS, NBD_FLAG_SEND_FUA) = -1 EBADF (%m)\n");
> +	ioctl(-1, NBD_SET_FLAGS, NBD_FLAG_SEND_TRIM);
> +	printf("ioctl(-1, NBD_SET_FLAGS, NBD_FLAG_SEND_TRIM) = -1 EBADF (%m)\n");
> +	ioctl(-1, NBD_SET_FLAGS, NBD_FLAG_CAN_MULTI_CONN);
> +	printf("ioctl(-1, NBD_SET_FLAGS, NBD_FLAG_CAN_MULTI_CONN) = -1 EBADF (%m)\n");

It is probably worth checking various flag combinations, including bits
for which there are no constants defined, in order to avoid situation like
v4.21~159.

ioctl(_IO(0xab, 0)) and ioctl(_IO(0xab, 11)) decoding is also probably worth
checking.


> diff --git a/xlat/nbd_ioctl_flags.in b/xlat/nbd_ioctl_flags.in
> new file mode 100644
> index 00000000..2040d221
> --- /dev/null
> +++ b/xlat/nbd_ioctl_flags.in
> @@ -0,0 +1,8 @@

> +NBD_FLAG_ROTATIONAL	(1 << 4)

> +NBD_FLAG_SEND_WRITE_ZEROES	(1 << 6)

Presence of flags absent in <linux/nbd.h> is probably worth some comment.


NBD_MAGIC is probably worth adding to xlat/fsmagic.in. I'm not sure,
however, as it's a private definition. (Funnily enough,
Documentation/process/magic-number.rst mentions that it is declared
under the name LO_MAGIC in the file "include/linux/nbd.h" which is not
true at all.)