| Message ID | 20200107165442.262020-2-hch@lst.de (mailing list archive) |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [1/4] xfs: clear kernel only flags in XFS_IOC_ATTRMULTI_BY_HANDLE | expand |
On Tue, Jan 07, 2020 at 05:54:39PM +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@lst.de> Looks ok, Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com> --D > --- > 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..edfbdb8f85e2 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; > + > 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..bd9d9ebf85d8 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.24.1 >
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..edfbdb8f85e2 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; + 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..bd9d9ebf85d8 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);
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@lst.de> --- 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(-)