On Thu, Dec 12, 2019 at 11:54:01AM +0100, Christoph Hellwig wrote:
> Don't allow passing arbitrary flags as they change behavior including
> memory allocation that the call stack is not prepared for.
>
> Fixes: ddbca70cc45c ("xfs: allocate xattr buffer on demand")
> Signed-off-by: Christoph Hellwig <hch@xxxxxx>
> ---
> fs/xfs/libxfs/xfs_attr.h | 7 +++++--
> fs/xfs/xfs_ioctl.c | 2 ++
> fs/xfs/xfs_ioctl32.c | 2 ++
> 3 files changed, 9 insertions(+), 2 deletions(-)
>
> diff --git a/fs/xfs/libxfs/xfs_attr.h b/fs/xfs/libxfs/xfs_attr.h
> index 94badfa1743e..91c2cb14276e 100644
> --- a/fs/xfs/libxfs/xfs_attr.h
> +++ b/fs/xfs/libxfs/xfs_attr.h
> @@ -26,7 +26,7 @@ struct xfs_attr_list_context;
> *========================================================================*/
>
>
> -#define ATTR_DONTFOLLOW 0x0001 /* -- unused, from IRIX -- */
> +#define ATTR_DONTFOLLOW 0x0001 /* -- ignored, from IRIX -- */
> #define ATTR_ROOT 0x0002 /* use attrs in root (trusted) namespace */
> #define ATTR_TRUST 0x0004 /* -- unused, from IRIX -- */
> #define ATTR_SECURE 0x0008 /* use attrs in security namespace */
> @@ -37,7 +37,10 @@ struct xfs_attr_list_context;
> #define ATTR_KERNOVAL 0x2000 /* [kernel] get attr size only, not value */
>
> #define ATTR_INCOMPLETE 0x4000 /* [kernel] return INCOMPLETE attr keys */
> -#define ATTR_ALLOC 0x8000 /* allocate xattr buffer on demand */
> +#define ATTR_ALLOC 0x8000 /* [kernel] allocate xattr buffer on demand */
> +
> +#define ATTR_KERNEL_FLAGS \
> + (ATTR_KERNOTIME | ATTR_KERNOVAL | ATTR_INCOMPLETE | ATTR_ALLOC)
>
> #define XFS_ATTR_FLAGS \
> { ATTR_DONTFOLLOW, "DONTFOLLOW" }, \
> diff --git a/fs/xfs/xfs_ioctl.c b/fs/xfs/xfs_ioctl.c
> index 7b35d62ede9f..2f76d0a7b818 100644
> --- a/fs/xfs/xfs_ioctl.c
> +++ b/fs/xfs/xfs_ioctl.c
> @@ -462,6 +462,8 @@ xfs_attrmulti_by_handle(
>
> error = 0;
> for (i = 0; i < am_hreq.opcount; i++) {
> + ops[i].am_flags &= ATTR_KERNEL_FLAGS;
The only flags we allow from userspace are the internal state flags?
Is this supposed to be am_flags &= ~ATTR_KERNEL_FLAGS?
--D
> +
> ops[i].am_error = strncpy_from_user((char *)attr_name,
> ops[i].am_attrname, MAXNAMELEN);
> if (ops[i].am_error == 0 || ops[i].am_error == MAXNAMELEN)
> diff --git a/fs/xfs/xfs_ioctl32.c b/fs/xfs/xfs_ioctl32.c
> index c4c4f09113d3..8b5acf8c42e1 100644
> --- a/fs/xfs/xfs_ioctl32.c
> +++ b/fs/xfs/xfs_ioctl32.c
> @@ -450,6 +450,8 @@ xfs_compat_attrmulti_by_handle(
>
> error = 0;
> for (i = 0; i < am_hreq.opcount; i++) {
> + ops[i].am_flags &= ATTR_KERNEL_FLAGS;
> +
> ops[i].am_error = strncpy_from_user((char *)attr_name,
> compat_ptr(ops[i].am_attrname),
> MAXNAMELEN);
> --
> 2.20.1
>
