[PATCH] file_ioctl: decode FS_IOC{32,}_{G,S}ETFLAGS ioctls

[Dmitry V. Levin]

Hi,

On Mon, Sep 02, 2019 at 11:35:50PM +0100, Pierre Marsais wrote:
> * xlat/inode_flags.in: New file.
> * file_ioctl.c: Include "xlat/inode_flags.h".
> (file_ioctl): Add decoding of FS_IOC_GETFLAGS, FS_IOC32_GETFLAGS,
> FS_IOC_SETFLAGS, and FS_IOC32_SETFLAGS.
> * tests/file_ioctl_inode_flags.c: New file.
> * tests/gen_tests.in (file_ioctl_inode_flags): New entry.
> * tests/pure_executables.list: Add file_ioctl_inode_flags.
> * tests/.gitignore: Likewise.
> 
> Signed-off-by: Pierre Marsais <pierre.marsais at lse.epita.fr>
> ---
> Hi strace developers,
> 
> Here is a simple patch decoding inode flags ioctls, as used by lsattr(1)
> and chattr(1), documented in ioctl_iflags(2).
> 
> I gathered all flags from linux source tree include/uapi/linux/fs.h to
> xlat/inode_flags.in, even though some of them should never be seen in
> userspace.
> 
>  file_ioctl.c                   | 19 ++++++++++++++
>  tests/.gitignore               |  1 +
>  tests/file_ioctl_inode_flags.c | 45 ++++++++++++++++++++++++++++++++++
>  tests/gen_tests.in             |  1 +
>  tests/pure_executables.list    |  1 +
>  xlat/inode_flags.in            | 28 +++++++++++++++++++++
>  6 files changed, 95 insertions(+)
>  create mode 100644 tests/file_ioctl_inode_flags.c
>  create mode 100644 xlat/inode_flags.in

Thanks for the patch, see my comments below.

> diff --git a/file_ioctl.c b/file_ioctl.c
> index 27ba4c82..f5601d23 100644
> --- a/file_ioctl.c
> +++ b/file_ioctl.c
> @@ -10,6 +10,8 @@
>  #include <linux/ioctl.h>
>  #include <linux/fs.h>
>  
> +#include "xlat/inode_flags.h"
> +
>  #ifdef HAVE_LINUX_FIEMAP_H
>  # include <linux/fiemap.h>
>  # include "xlat/fiemap_flags.h"

> @@ -175,6 +177,23 @@ file_ioctl(struct tcb *const tcp, const unsigned int code,
>  		return 0;
>  	}
>  
> +	case FS_IOC_GETFLAGS:
> +	case FS_IOC32_GETFLAGS:
> +		if (entering(tcp))
> +			return 0;
> +		ATTRIBUTE_FALLTHROUGH;
> +	case FS_IOC_SETFLAGS:
> +	case FS_IOC32_SETFLAGS: {
> +		uint32_t flags;
> +
> +		tprints(", ");
> +		if (umove_or_printaddr(tcp, arg, &flags))
> +			break;
> +
> +		printflags(inode_flags, flags, "FS_???_FL");
> +		break;
> +	}

These flags should be printed in square brackets to reflect the indirection.

> new file mode 100644
> index 00000000..c2fc49fb
> --- /dev/null
> +++ b/tests/file_ioctl_inode_flags.c
> @@ -0,0 +1,45 @@
> +/*
> + * Copyright (c) 2019 The strace developers.
> + * All rights reserved.
> + *
> + * SPDX-License-Identifier: GPL-2.0-or-later
> + */
> +
> +#include "tests.h"
> +
> +# include <stdio.h>
> +# include <sys/ioctl.h>
> +# include <linux/fs.h>
> +# include "xlat.h"
> +# include "xlat/inode_flags.h"

What does this extra level of indentation mean?


-- 
ldv