[PATCH] Add support for /dev/[u]random ioctls

[Dmitry V. Levin]

Hi,

On Fri, Nov 02, 2018 at 08:00:19AM +0000, Rasmus Villemoes wrote:
> * random_ioctl.c: New file.
> * Makefile.am (strace_SOURCES): Add it.
> * defs.h (DECL_IOCTL): Add random.
> * ioctl.c (ioctl_decode): Add 'R' case.
> 
> Signed-off-by: Rasmus Villemoes <rasmus.villemoes at prevas.dk>

Thanks for the patch, see my comments below.

> ---
>  Makefile.am    |  1 +
>  defs.h         |  1 +
>  ioctl.c        |  2 ++
>  random_ioctl.c | 54 ++++++++++++++++++++++++++++++++++++++++++++++++++
>  4 files changed, 58 insertions(+)
>  create mode 100644 random_ioctl.c
> 
> diff --git a/Makefile.am b/Makefile.am
> index 913d26a9..0680763e 100644
> --- a/Makefile.am
> +++ b/Makefile.am
> @@ -267,6 +267,7 @@ strace_SOURCES =	\
>  	ptp.c		\
>  	ptrace.h	\
>  	quota.c		\
> +	random_ioctl.c	\
>  	readahead.c	\
>  	readlink.c	\
>  	reboot.c	\
> diff --git a/defs.h b/defs.h
> index 5ba95f53..b8d561aa 100644
> --- a/defs.h
> +++ b/defs.h
> @@ -973,6 +973,7 @@ DECL_IOCTL(kvm);
>  DECL_IOCTL(nbd);
>  DECL_IOCTL(nsfs);
>  DECL_IOCTL(ptp);
> +DECL_IOCTL(random);
>  DECL_IOCTL(scsi);
>  DECL_IOCTL(term);
>  DECL_IOCTL(ubi);
> diff --git a/ioctl.c b/ioctl.c
> index 4c9e7db3..e7dd4c4e 100644
> --- a/ioctl.c
> +++ b/ioctl.c
> @@ -329,6 +329,8 @@ ioctl_decode(struct tcb *tcp)
>  		return inotify_ioctl(tcp, code, arg);
>  	case 0xab:
>  		return nbd_ioctl(tcp, code, arg);
> +	case 'R':
> +		return random_ioctl(tcp, code, arg);
>  	default:
>  		break;
>  	}
> diff --git a/random_ioctl.c b/random_ioctl.c
> new file mode 100644
> index 00000000..8acbc34b
> --- /dev/null
> +++ b/random_ioctl.c
> @@ -0,0 +1,54 @@
> +
> +#include "defs.h"
> +
> +#include <linux/random.h>
> +
> +/*
> + * RNDGETPOOL was removed in 2.6.9, so non-ancient kernels always
> + * return -EINVAL for that.
> + */
> +
> +int
> +random_ioctl(struct tcb *const tcp, const unsigned int code,
> +	   const kernel_ulong_t arg)
> +{
> +	struct rand_pool_info info;
> +	int count;
> +	
> +	switch (code) {
> +	case RNDGETENTCNT:
> +		if (entering(tcp))
> +			return 0;
> +		ATTRIBUTE_FALLTHROUGH;
> +	case RNDADDTOENTCNT:
> +		tprints(", ");
> +		if (umove_or_printaddr(tcp, arg, &count))
> +			return RVAL_IOCTL_DECODED;
> +		tprintf("entropy_count=%d", count);

The corresponding output would be
	ioctl(%d, RNDADDTOENTCNT, entropy_count=%d)
which would look very unusual.

The proper way to print this kind of ioctl commands is
	printnum_int(tcp, arg, "%d");

> +		break;
> +		
> +	case RNDADDENTROPY:
> +		tprints(", ");
> +		if (umove_or_printaddr(tcp, arg, &info))
> +			return RVAL_IOCTL_DECODED;
> +		/* 
> +		 * It might be nice to print a few bytes from the
> +		 * ->buf, but we don't that we can even fetch one byte
> +		 * beyond until we know buf_size. So keep it simple.
> +		 */
> +		tprintf("{entropy_count=%d, buf_size=%d}",
> +			info.entropy_count, info.buf_size);

You can use PRINT_FIELD_D macro instead, e.g.

	if (!umove_or_printaddr(tcp, arg, &info)) {
		PRINT_FIELD_D("{", info, entropy_count);
		PRINT_FIELD_D(", ", info, buf_size);
		tprints("}");
	}

As the buf_size field has already been fetched, the array can be printed, too:
	PRINT_FIELD_STRN(", ", info, buf, info.buf_size, tcp);

> +		break;
> +		
> +	/* ioctls with no parameters */
> +	case RNDZAPENTCNT:
> +	case RNDCLEARPOOL:
> +#ifdef RNDRESEEDCRNG /* only linux 4.17+ */
> +	case RNDRESEEDCRNG:
> +#endif
> +		break;
> +	default:
> +		return RVAL_DECODED;
> +	}
> +	return RVAL_IOCTL_DECODED;
> +}

As a general rule, all new decoders should be accompanied by tests.
The most recently added ioctl decoder (commit v4.25~3) comes with a test,
you can use it and other ioctl tests as examples.


-- 
ldv
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 801 bytes
Desc: not available
URL: <http://lists.strace.io/pipermail/strace-devel/attachments/20181105/4fa9eeae/attachment.bin>