<div dir="ltr">Thank you for review. I have rewritten my code, but I still have some problems with NS_GET_NSTYPE parsing without additional argument. And new tests are not fully tested, because I haven't build kernel yet.</div><div class="gmail_extra"><br><div class="gmail_quote">On Thu, Mar 16, 2017 at 9:01 AM, Dmitry V. Levin <span dir="ltr"><<a href="mailto:ldv@altlinux.org" target="_blank">ldv@altlinux.org</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><span class="">On Wed, Mar 15, 2017 at 03:22:36PM +0700, Nikolay Marchuk wrote:<br>
> Hello,<br>
> I have implemented NS_* ioctls. Should I use common ioctlent interface to<br>
> print them with additional argument or should I print them without it?<br>
<br>
</span>Why do you think printing additional arguments makes any sense?<br>
<br>
> From f03f6cea9ff2f1ca6fc38c00688446<wbr>58180c5032 Mon Sep 17 00:00:00 2001<br>
> From: Marchuk Nikolay <<a href="mailto:marchuk.nikolay.a@gmail.com">marchuk.nikolay.a@gmail.com</a>><br>
> Date: Wed, 15 Mar 2017 11:59:26 +0700<br>
> Subject: [PATCH 2/2] Add ioctl namespace entries from Linux 4.11<br>
><br>
> Signed-off-by: Nikolay Marchuk <<a href="mailto:marchuk.nikolay.a@gmail.com">marchuk.nikolay.a@gmail.com</a>><br>
> ---<br>
>  Makefile.am           |  1 +<br>
>  <a href="http://configure.ac" rel="noreferrer" target="_blank">configure.ac</a>          |  1 +<br>
>  defs.h                |  1 +<br>
>  ioctl.c               |  4 +++<br>
>  nsfs.c                | 38 ++++++++++++++++++++++++++++<br>
>  tests/.gitignore      |  1 +<br>
>  tests/Makefile.am     |  2 ++<br>
>  tests/ioctl_nsfs.c    | 70 ++++++++++++++++++++++++++++++<wbr>+++++++++++++++++++++<br>
>  tests/ioctl_nsfs.test | 13 ++++++++++<br>
>  xlat/<a href="http://nsfs_types.in" rel="noreferrer" target="_blank">nsfs_types.in</a>    |  6 +++++<br>
>  10 files changed, 137 insertions(+)<br>
>  create mode 100644 nsfs.c<br>
>  create mode 100644 tests/ioctl_nsfs.c<br>
>  create mode 100755 tests/ioctl_nsfs.test<br>
>  create mode 100644 xlat/<a href="http://nsfs_types.in" rel="noreferrer" target="_blank">nsfs_types.in</a><br>
><br>
> diff --git a/Makefile.am b/Makefile.am<br>
> index c77f463..c673faa 100644<br>
> --- a/Makefile.am<br>
> +++ b/Makefile.am<br>
> @@ -172,6 +172,7 @@ strace_SOURCES =  \<br>
>       net.c           \<br>
>       netlink.c       \<br>
>       nsig.h          \<br>
> +     nsfs.c      \<br>
<br>
This doesn't look nice.<br>
<br>
>       numa.c          \<br>
>       oldstat.c       \<br>
>       open.c          \<br>
> diff --git a/<a href="http://configure.ac" rel="noreferrer" target="_blank">configure.ac</a> b/<a href="http://configure.ac" rel="noreferrer" target="_blank">configure.ac</a><br>
> index 9e5087b..dc49fdc 100644<br>
> --- a/<a href="http://configure.ac" rel="noreferrer" target="_blank">configure.ac</a><br>
> +++ b/<a href="http://configure.ac" rel="noreferrer" target="_blank">configure.ac</a><br>
> @@ -366,6 +366,7 @@ AC_CHECK_HEADERS(m4_normalize(<wbr>[<br>
>       linux/ipc.h<br>
>       linux/mmtimer.h<br>
>       linux/msg.h<br>
> +     linux/nsfs.h<br>
>       linux/perf_event.h<br>
>       linux/quota.h<br>
>       linux/seccomp.h<br>
> diff --git a/defs.h b/defs.h<br>
> index 793971e..0f3ec14 100644<br>
> --- a/defs.h<br>
> +++ b/defs.h<br>
> @@ -640,6 +640,7 @@ name ## _ioctl(struct tcb *, unsigned int request, kernel_ulong_t arg)<br>
>  DECL_IOCTL(dm);<br>
>  DECL_IOCTL(file);<br>
>  DECL_IOCTL(fs_x);<br>
> +DECL_IOCTL(nsfs);<br>
>  DECL_IOCTL(ptp);<br>
>  DECL_IOCTL(scsi);<br>
>  DECL_IOCTL(term);<br>
> diff --git a/ioctl.c b/ioctl.c<br>
> index aa1880f..d972c38 100644<br>
> --- a/ioctl.c<br>
> +++ b/ioctl.c<br>
> @@ -280,6 +280,10 @@ ioctl_decode(struct tcb *tcp)<br>
>       case 0x94:<br>
>               return btrfs_ioctl(tcp, code, arg);<br>
>  #endif<br>
> +#ifdef HAVE_LINUX_NSFS_H<br>
> +     case 0xb7:<br>
> +             return nsfs_ioctl(tcp, code, arg);<br>
> +#endif<br>
>  #ifdef HAVE_LINUX_DM_IOCTL_H<br>
>       case 0xfd:<br>
>               return dm_ioctl(tcp, code, arg);<br>
> diff --git a/nsfs.c b/nsfs.c<br>
> new file mode 100644<br>
> index 0000000..de98ca9<br>
> --- /dev/null<br>
> +++ b/nsfs.c<br>
> @@ -0,0 +1,38 @@<br>
> +#include "defs.h"<br>
> +<br>
> +#ifdef HAVE_LINUX_NSFS_H<br>
> +<br>
> +#include "xlat/nsfs_types.h"<br>
> +#include <linux/ioctl.h><br>
> +#include <linux/nsfs.h><br>
> +<br>
> +/* Definitions for command which have been added later */<br>
> +#ifndef NS_GET_NSTYPE<br>
> +#define NS_GET_NSTYPE                _IO(NSIO, 0x3)<br>
> +#endif<br>
> +#ifndef NS_GET_OWNER_UID<br>
> +#define NS_GET_OWNER_UID     _IO(NSIO, 0x4)<br>
> +#endif<br>
<br>
Please indent.<br>
<br>
> +<br>
> +int<br>
> +nsfs_ioctl(struct tcb * tcp, unsigned int code, kernel_ulong_t arg){<br>
> +     char *outstr;<br>
> +     if (entering(tcp) || syserror(tcp))<br>
> +             return 0;<br>
<br>
This way the 3rd argument of ioctl syscall is going to be printed.<br>
<br>
> +     switch(code){<br>
> +             case NS_GET_USERNS:<br>
> +             case NS_GET_PARENT:<br>
> +                     return RVAL_DECODED | RVAL_FD;<br>
<br>
This way the 3rd argument of ioctl syscall is going to be printed.<br>
<br>
I'm not sure you've got the ioctl return code semantics right,<br>
please read <a href="https://sourceforge.net/p/strace/mailman/message/34451172/" rel="noreferrer" target="_blank">https://sourceforge.net/p/<wbr>strace/mailman/message/<wbr>34451172/</a><br>
<br>
I think these 2 operation can (and should) be decoded on entering rather<br>
than on exiting.<br>
<br>
> +             case NS_GET_OWNER_UID:<br>
> +                     return RVAL_UDECIMAL | RVAL_DECODED;<br>
<br>
The semantics of NS_GET_OWNER_UID is different: the uid is written<br>
to the memory at address "arg", see linux fs/nsfs.c.<br>
<br>
> +             case NS_GET_NSTYPE:<br>
> +                     outstr = xlookup(nsfs_types, tcp->u_rval);<br>
> +                     if(outstr){<br>
> +                             tcp->auxstr = outstr;<br>
> +                             return RVAL_STR | RVAL_DECODED;<br>
> +                     }<br>
<br>
This way the 3rd argument of ioctl syscall is going to be printed.<br>
<br>
> +             default:<br>
> +                     return 0;<br>
> +     }<br>
> +}<br>
> +#endif /* HAVE_LINUX_NSFS_H */<br>
> diff --git a/tests/.gitignore b/tests/.gitignore<br>
> index a7754b6..060a391 100644<br>
> --- a/tests/.gitignore<br>
> +++ b/tests/.gitignore<br>
> @@ -121,6 +121,7 @@ ioctl_loop<br>
>  ioctl_loop-nv<br>
>  ioctl_loop-v<br>
>  ioctl_mtd<br>
> +ioctl_nsfs<br>
>  ioctl_rtc<br>
>  ioctl_rtc-v<br>
>  ioctl_scsi<br>
> diff --git a/tests/Makefile.am b/tests/Makefile.am<br>
> index c5c124c..bc0c621 100644<br>
> --- a/tests/Makefile.am<br>
> +++ b/tests/Makefile.am<br>
> @@ -184,6 +184,7 @@ check_PROGRAMS = \<br>
>       ioctl_loop-nv \<br>
>       ioctl_loop-v \<br>
>       ioctl_mtd \<br>
> +     ioctl_nsfs \<br>
>       ioctl_rtc \<br>
>       ioctl_rtc-v \<br>
>       ioctl_scsi \<br>
> @@ -593,6 +594,7 @@ DECODER_TESTS = \<br>
>       ioctl_loop-v.test \<br>
>       ioctl_loop.test \<br>
>       ioctl_mtd.test \<br>
> +     ioctl_nsfs.test \<br>
>       ioctl_rtc-v.test \<br>
>       ioctl_rtc.test \<br>
>       ioctl_scsi.test \<br>
> diff --git a/tests/ioctl_nsfs.c b/tests/ioctl_nsfs.c<br>
> new file mode 100644<br>
> index 0000000..beea729<br>
> --- /dev/null<br>
> +++ b/tests/ioctl_nsfs.c<br>
> @@ -0,0 +1,70 @@<br>
> +/*<br>
> + * Check decoding of NS_* commands of ioctl syscall.<br>
> + *<br>
> + * Copyright (c) 2017 Nikolay Marchuk <<a href="mailto:marchuk.nikolay.a@gmail.com">marchuk.nikolay.a@gmail.com</a>><br>
> + * All rights reserved.<br>
> + *<br>
> + * Redistribution and use in source and binary forms, with or without<br>
> + * modification, are permitted provided that the following conditions<br>
> + * are met:<br>
> + * 1. Redistributions of source code must retain the above copyright<br>
> + *    notice, this list of conditions and the following disclaimer.<br>
> + * 2. Redistributions in binary form must reproduce the above copyright<br>
> + *    notice, this list of conditions and the following disclaimer in the<br>
> + *    documentation and/or other materials provided with the distribution.<br>
> + * 3. The name of the author may not be used to endorse or promote products<br>
> + *    derived from this software without specific prior written permission.<br>
> + *<br>
> + * THIS SOFTWARE IS PROVIDED BY THE AUTHOR ``AS IS'' AND ANY EXPRESS OR<br>
> + * IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES<br>
> + * OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE DISCLAIMED.<br>
> + * IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR ANY DIRECT, INDIRECT,<br>
> + * INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT<br>
> + * NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,<br>
> + * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY<br>
> + * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT<br>
> + * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF<br>
> + * THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.<br>
> + */<br>
> +<br>
> +#include "tests.h"<br>
> +<br>
> +#ifdef HAVE_LINUX_NSFS_H<br>
> +<br>
> +#include <stdio.h><br>
> +#include <sys/ioctl.h><br>
> +#include <linux/ioctl.h><br>
> +#include <linux/nsfs.h><br>
> +<br>
> +/* Definitions for command which have been added later */<br>
> +#ifndef NS_GET_NSTYPE<br>
> +#define NS_GET_NSTYPE                _IO(NSIO, 0x3)<br>
> +#endif<br>
> +#ifndef NS_GET_OWNER_UID<br>
> +#define NS_GET_OWNER_UID     _IO(NSIO, 0x4)<br>
> +#endif<br>
<br>
Please indent.<br>
<br>
> +<br>
> +int<br>
> +main(void)<br>
> +{<br>
> +     static kernel_ulong_t dummy_arg =<br>
> +             (kernel_ulong_t) 0xbadc0dedda7a1057ULL;<br>
> +     ioctl(-1, NS_GET_USERNS, dummy_arg);<br>
> +     printf("ioctl(-1, NS_GET_USERNS, %#lx) = -1 EBADF (%m)\n", dummy_arg);<br>
> +     ioctl(-1, NS_GET_PARENT, dummy_arg);<br>
> +     printf("ioctl(-1, NS_GET_PARENT, %#lx) = -1 EBADF (%m)\n", dummy_arg);<br>
> +     ioctl(-1, NS_GET_NSTYPE, dummy_arg);<br>
> +     printf("ioctl(-1, NS_GET_NSTYPE, %#lx) = -1 EBADF (%m)\n", dummy_arg);<br>
> +     ioctl(-1, NS_GET_OWNER_UID, dummy_arg);<br>
> +     printf("ioctl(-1, NS_GET_OWNER_UID, %#lx) = -1 EBADF (%m)\n", dummy_arg);<br>
> +<br>
> +     puts("+++ exited with 0 +++");<br>
> +     return 0;<br>
> +}<br>
<br>
This way you test the generic parser, the most part of your parser (the<br>
switch) is not going to be covered.  Please add some calls with valid ns<br>
descriptors, too.<br>
<br>
> +<br>
> +<br>
> +#else /* !HAVE_LINUX_DM_IOCTL_H */<br>
> +<br>
> +SKIP_MAIN_UNDEFINED("HAVE_<wbr>LINUX_NSFS_H")<br>
> +<br>
> +#endif /* HAVE_LINUX_DM_IOCTL_H */<br>
> \ No newline at end of file<br>
> diff --git a/tests/ioctl_nsfs.test b/tests/ioctl_nsfs.test<br>
> new file mode 100755<br>
> index 0000000..b4604ed<br>
> --- /dev/null<br>
> +++ b/tests/ioctl_nsfs.test<br>
> @@ -0,0 +1,13 @@<br>
> +#!/bin/sh<br>
> +<br>
> +# Check decoding of NS_* ioctls.<br>
> +<br>
> +. "${srcdir=.}/init.sh"<br>
> +<br>
> +run_prog > /dev/null<br>
> +run_strace -a16 -eioctl $args > "$EXP"<br>
> +check_prog grep<br>
> +grep -v '^ioctl([012],' < "$LOG" > "$OUT"<br>
> +match_diff "$OUT" "$EXP"<br>
> +<br>
> +rm -f "$EXP" "$OUT"<br>
> \ No newline at end of file<br>
<br>
Please add a newline.<br>
<br>
> diff --git a/xlat/<a href="http://nsfs_types.in" rel="noreferrer" target="_blank">nsfs_types.in</a> b/xlat/<a href="http://nsfs_types.in" rel="noreferrer" target="_blank">nsfs_types.in</a><br>
> new file mode 100644<br>
> index 0000000..249935e<br>
> --- /dev/null<br>
> +++ b/xlat/<a href="http://nsfs_types.in" rel="noreferrer" target="_blank">nsfs_types.in</a><br>
> @@ -0,0 +1,6 @@<br>
> +CLONE_NEWCGROUP      0x02000000<br>
> +CLONE_NEWUTS 0x04000000<br>
> +CLONE_NEWIPC 0x08000000<br>
> +CLONE_NEWUSER        0x10000000<br>
> +CLONE_NEWPID 0x20000000<br>
> +CLONE_NEWNET 0x40000000<br>
<br>
xlat/<a href="http://setns_types.in" rel="noreferrer" target="_blank">setns_types.in</a> looks very similar to this file, do you really need it?<br>
<span class="HOEnZb"><font color="#888888"><br>
<br>
--<br>
ldv<br>
</font></span><br>------------------------------<wbr>------------------------------<wbr>------------------<br>
Check out the vibrant tech community on one of the world's most<br>
engaging tech sites, Slashdot.org! <a href="http://sdm.link/slashdot" rel="noreferrer" target="_blank">http://sdm.link/slashdot</a><br>______________________________<wbr>_________________<br>
Strace-devel mailing list<br>
<a href="mailto:Strace-devel@lists.sourceforge.net">Strace-devel@lists.<wbr>sourceforge.net</a><br>
<a href="https://lists.sourceforge.net/lists/listinfo/strace-devel" rel="noreferrer" target="_blank">https://lists.sourceforge.net/<wbr>lists/listinfo/strace-devel</a><br>
<br></blockquote></div><br></div>