RFC userfaultfd ioctl decode

[Dmitry V. Levin]

On Thu, Apr 21, 2016 at 02:04:33PM +0100, Dr. David Alan Gilbert wrote:
[...]
> --- a/configure.ac
> +++ b/configure.ac
> @@ -420,6 +420,9 @@ AC_CHECK_HEADERS([linux/input.h], [
>  	AC_CHECK_MEMBERS([struct input_absinfo.resolution],,, [#include <linux/input.h>])
>  ])
>  
> +AC_CHECK_HEADERS([linux/userfaultfd.h], [
> +])

This should rather go to the first (big) AC_CHECK_HEADERS list
in the file.

> --- a/ioctl.c
> +++ b/ioctl.c
> @@ -263,6 +263,8 @@ ioctl_decode(struct tcb *tcp)
>  	case 'E':
>  		return evdev_ioctl(tcp, code, arg);
>  #endif
> +	case 0xaa:
> +		return uffdio_ioctl(tcp, code, arg);

The tradition approach is to wrap this into
#ifdef HAVE_LINUX_USERFAULTFD_H / #endif
so that ...

> --- a/userfaultfd.c
> +++ b/userfaultfd.c
> @@ -27,9 +27,50 @@
>  
>  #include "defs.h"
>  #include <fcntl.h>
> +#include <linux/ioctl.h>
> +#ifdef HAVE_LINUX_USERFAULTFD_H
> +#include <linux/userfaultfd.h>
> +#endif
>  
>  #include "xlat/uffd_flags.h"
>  
> +
> +int
> +uffdio_ioctl(struct tcb *tcp, const unsigned int code, const long arg)
> +{
> +#ifdef HAVE_LINUX_USERFAULTFD_H

... the whole parser with all headers it needs could be wrapped
with a single ifdef.

> +	case UFFDIO_COPY: {
> +		struct uffdio_copy uc;
> +		if (!umove_or_printaddr(tcp, arg, &uc)) {
> +			if (entering(tcp)) {
> +				tprintf("{dst=%" PRIx64 ", src=%" PRIx64
> +				", len=%" PRIu64 ", mode=%" PRIu64 "}",
> +				(uint64_t)uc.dst, (uint64_t)uc.src,
> +				(uint64_t)uc.len, (uint64_t)uc.mode);

I suppose you can use recently added PRI__*64 instead of PRI*64,
this way you won't need explicit casts.

> +				return 1;
> +			} else {
> +				/* copy also returns the number of bytes
> +				 * copied */
> +				sprintf(auxbuf, "copy=%" PRId64,
> +				(int64_t)uc.copy);
> +				tcp->auxstr = auxbuf;
> +				return RVAL_STR | 1;
> +			}

The preferred method is to print uc.copy as a part of structure:

	if (entering(tcp)) {
		if (umove_or_printaddr(tcp, arg, &uc))
			return RVAL_DECODED | 1;
		tprintf("{dst=%" PRI__x64 ", src=%" PRI__x64
			", len=%" PRI__u64 ", mode=%" PRI__u64
			", copy=",
			uc.dst, uc.src, uc.len, uc.mode);
			return 1;
	} else {
		if (!umove_or_printaddr(tcp, arg, &uc.copy))
			tprintf("%" PRI__d64, uc.copy);
		tprints("}");
		return 1;
	}

> +		}
> +		break;

As (your edition of) this parser has already printed something at this
point, it has to return 1 + RVAL_something to let the caller know,
otherwise the address will be printed twice.  You can easily reproduce
this by invoking UFFDIO_COPY with NULL address.

> +	}
> +#endif
> +
> +	default:
> +		return 0;

As this parser in default case is not going to do anything on exiting,
it report this to the caller by returning RVAL_DECODED.


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