[4/3] generic: posix acl extended attribute memory corruption test
diff mbox series

Message ID 20190213204814.GB6477@magnolia
State Accepted
Headers show
Series
  • fstests: fixes and new tests
Related show

Commit Message

Darrick J. Wong Feb. 13, 2019, 8:48 p.m. UTC
From: Darrick J. Wong <darrick.wong@oracle.com>

XFS had a use-after-free bug when xfs_xattr_put_listent runs out of
listxattr buffer space while trying to store the name
"system.posix_acl_access" and then corrupts memory by not checking the
seen_enough state and then trying to shove "trusted.SGI_ACL_FILE" into
the buffer as well.

In order to tickle the bug in a user visible way we must have already
put a name in the buffer, so we take advantage of the fact that
"security.evm" sorts before "system.posix_acl_access" to make sure this
happens.

Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
 .gitignore              |    1 
 src/Makefile            |    2 -
 src/t_attr_corruption.c |  122 +++++++++++++++++++++++++++++++++++++++++++++++
 tests/generic/712       |   41 ++++++++++++++++
 tests/generic/712.out   |    2 +
 tests/generic/group     |    1 
 6 files changed, 168 insertions(+), 1 deletion(-)
 create mode 100644 src/t_attr_corruption.c
 create mode 100755 tests/generic/712
 create mode 100644 tests/generic/712.out

Comments

Eryu Guan Feb. 16, 2019, 12:05 p.m. UTC | #1
On Wed, Feb 13, 2019 at 12:48:14PM -0800, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@oracle.com>
> 
> XFS had a use-after-free bug when xfs_xattr_put_listent runs out of
> listxattr buffer space while trying to store the name
> "system.posix_acl_access" and then corrupts memory by not checking the
> seen_enough state and then trying to shove "trusted.SGI_ACL_FILE" into
> the buffer as well.
> 
> In order to tickle the bug in a user visible way we must have already
> put a name in the buffer, so we take advantage of the fact that
> "security.evm" sorts before "system.posix_acl_access" to make sure this
> happens.
> 
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> ---
>  .gitignore              |    1 
>  src/Makefile            |    2 -
>  src/t_attr_corruption.c |  122 +++++++++++++++++++++++++++++++++++++++++++++++
>  tests/generic/712       |   41 ++++++++++++++++
>  tests/generic/712.out   |    2 +
>  tests/generic/group     |    1 
>  6 files changed, 168 insertions(+), 1 deletion(-)
>  create mode 100644 src/t_attr_corruption.c
>  create mode 100755 tests/generic/712
>  create mode 100644 tests/generic/712.out
> 
> diff --git a/.gitignore b/.gitignore
> index ea1aac8a..0933dc7d 100644
> --- a/.gitignore
> +++ b/.gitignore
> @@ -114,6 +114,7 @@
>  /src/stat_test
>  /src/swapon
>  /src/t_access_root
> +/src/t_attr_corruption
>  /src/t_dir_offset
>  /src/t_dir_offset2
>  /src/t_dir_type
> diff --git a/src/Makefile b/src/Makefile
> index 41826585..ae09eb0a 100644
> --- a/src/Makefile
> +++ b/src/Makefile
> @@ -27,7 +27,7 @@ LINUX_TARGETS = xfsctl bstat t_mtab getdevicesize preallo_rw_pattern_reader \
>  	renameat2 t_getcwd e4compact test-nextquota punch-alternating \
>  	attr-list-by-handle-cursor-test listxattr dio-interleaved t_dir_type \
>  	dio-invalidate-cache stat_test t_encrypted_d_revalidate \
> -	attr_replace_test swapon mkswap
> +	attr_replace_test swapon mkswap t_attr_corruption
>  
>  SUBDIRS = log-writes perf
>  
> diff --git a/src/t_attr_corruption.c b/src/t_attr_corruption.c
> new file mode 100644
> index 00000000..1fa5e41f
> --- /dev/null
> +++ b/src/t_attr_corruption.c
> @@ -0,0 +1,122 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * Copyright (C) 2019 Oracle.  All Rights Reserved.
> + * Author: Darrick J. Wong <darrick.wong@oracle.com>
> + *
> + * Test program to tickle a use-after-free bug in xfs.
> + *
> + * XFS had a use-after-free bug when xfs_xattr_put_listent runs out of
> + * listxattr buffer space while trying to store the name
> + * "system.posix_acl_access" and then corrupts memory by not checking the
> + * seen_enough state and then trying to shove "trusted.SGI_ACL_FILE" into the
> + * buffer as well.
> + *
> + * In order to tickle the bug in a user visible way we must have already put a
> + * name in the buffer, so we take advantage of the fact that "security.evm"
> + * sorts before "system.posix_acl_access" to make sure this happens.
> + *
> + * If we trigger the bug, the program will print the garbled string
> + * "rusted.SGI_ACL_FILE".  If the bug is fixed, the flistxattr call returns
> + * ERANGE.
> + */
> +#include <sys/types.h>
> +#include <sys/stat.h>
> +#include <fcntl.h>
> +#include <stdlib.h>
> +#include <stdio.h>
> +#include <string.h>
> +#include <stdint.h>
> +#include <unistd.h>
> +#include <attr/xattr.h>
> +
> +void die(const char *msg)
> +{
> +	perror(msg);
> +	exit(1);
> +}
> +
> +struct entry {
> +	uint16_t	a;
> +	uint16_t	b;
> +	uint32_t	c;
> +};
> +
> +struct myacl {
> +	uint32_t	d;
> +	struct entry	e[4];
> +};
> +
> +int main(int argc, char *argv[])
> +{
> +	struct myacl acl = {
> +		.d = 2,
> +		.e = {
> +			{1, 0, 0},
> +			{4, 0, 0},
> +			{0x10, 0, 0},
> +			{0x20, 0, 0},
> +		},
> +	};
> +	char buf[64];
> +	ssize_t sz;
> +	int fd;
> +	int ret;
> +
> +	if (argc > 1) {
> +		ret = chdir(argv[1]);
> +		if (ret)
> +			die(argv[1]);
> +	}
> +
> +	fd = creat("file0", 0644);
> +	if (fd < 0)
> +		die("create");
> +
> +	ret = fsetxattr(fd, "system.posix_acl_access", &acl, sizeof(acl), 0);
> +	if (ret)
> +		die("set posix acl");
> +
> +	ret = fsetxattr(fd, "security.evm", buf, 1, 1);
> +	if (ret)
> +		die("set evm");
> +
> +	sz = flistxattr(fd, buf, 30);
> +	if (sz < 0)
> +		die("list attr");
> +
> +	printf("%s\n", buf);
> +
> +	return 0;
> +
> +#if 0
> +	/* original syzkaller reproducer */
> +
> +	syscall(__NR_mmap, 0x20000000, 0x1000000, 3, 0x32, -1, 0);
> +
> +	memcpy((void*)0x20000180, "./file0", 8);
> +	syscall(__NR_creat, 0x20000180, 0);
> +	memcpy((void*)0x20000000, "./file0", 8);
> +	memcpy((void*)0x20000040, "system.posix_acl_access", 24);
> +	*(uint32_t*)0x20000680 = 2;
> +	*(uint16_t*)0x20000684 = 1;
> +	*(uint16_t*)0x20000686 = 0;
> +	*(uint32_t*)0x20000688 = 0;
> +	*(uint16_t*)0x2000068c = 4;
> +	*(uint16_t*)0x2000068e = 0;
> +	*(uint32_t*)0x20000690 = 0;
> +	*(uint16_t*)0x20000694 = 0x10;
> +	*(uint16_t*)0x20000696 = 0;
> +	*(uint32_t*)0x20000698 = 0;
> +	*(uint16_t*)0x2000069c = 0x20;
> +	*(uint16_t*)0x2000069e = 0;
> +	*(uint32_t*)0x200006a0 = 0;
> +	syscall(__NR_setxattr, 0x20000000, 0x20000040, 0x20000680, 0x24, 0);
> +	memcpy((void*)0x20000080, "./file0", 8);
> +	memcpy((void*)0x200000c0, "security.evm", 13);
> +	memcpy((void*)0x20000100, "\x03\x00\x00\x00\x57", 5);
> +	syscall(__NR_lsetxattr, 0x20000080, 0x200000c0, 0x20000100, 1, 1);
> +	memcpy((void*)0x20000300, "./file0", 8);
> +	syscall(__NR_listxattr, 0x20000300, 0x200002c0, 0x1e);
> +	return 0;
> +#endif
> +}
> diff --git a/tests/generic/712 b/tests/generic/712
> new file mode 100755
> index 00000000..6348a797
> --- /dev/null
> +++ b/tests/generic/712
> @@ -0,0 +1,41 @@
> +#! /bin/bash
> +# SPDX-License-Identifier: GPL-2.0+
> +# Copyright (c) 2019 Oracle, Inc.  All Rights Reserved.
> +#
> +# FS QA Test No. 712
> +#
> +# Regression test for a bug where XFS corrupts memory if the listxattr buffer
> +# is a particularly well crafted size on a filesystem that supports posix acls.
> +#
> +seq=`basename $0`
> +seqres=$RESULT_DIR/$seq
> +echo "QA output created by $seq"
> +tmp=/tmp/$$
> +status=1	# failure is the default!
> +testfile=$TEST_DIR/$seq.txt

I removed this definition, which is not used in the test.

> +trap "_cleanup; exit \$status" 0 1 2 3 15
> +
> +_cleanup()
> +{
> +	cd /
> +	rm -f $tmp.*
> +}
> +
> +# get standard environment, filters and checks
> +. ./common/rc
> +. ./common/attr
> +
> +# real QA test starts here
> +_supported_fs generic
> +_supported_os Linux
> +_require_acls
> +_require_scratch

I also added

_require_test_program "t_attr_corruption"

Thanks,
Eryu

> +
> +rm -f $seqres.full
> +_scratch_mkfs >> $seqres.full 2>&1
> +_scratch_mount
> +
> +src/t_attr_corruption $SCRATCH_MNT
> +
> +status=0
> +exit
> diff --git a/tests/generic/712.out b/tests/generic/712.out
> new file mode 100644
> index 00000000..a2ba09f3
> --- /dev/null
> +++ b/tests/generic/712.out
> @@ -0,0 +1,2 @@
> +QA output created by 712
> +list attr: Numerical result out of range
> diff --git a/tests/generic/group b/tests/generic/group
> index f56eb475..b3086154 100644
> --- a/tests/generic/group
> +++ b/tests/generic/group
> @@ -529,3 +529,4 @@
>  524 auto quick
>  525 auto quick rw
>  709 auto quick
> +712 auto quick attr
Darrick J. Wong Feb. 16, 2019, 5:24 p.m. UTC | #2
On Sat, Feb 16, 2019 at 08:05:30PM +0800, Eryu Guan wrote:
> On Wed, Feb 13, 2019 at 12:48:14PM -0800, Darrick J. Wong wrote:
> > From: Darrick J. Wong <darrick.wong@oracle.com>
> > 
> > XFS had a use-after-free bug when xfs_xattr_put_listent runs out of
> > listxattr buffer space while trying to store the name
> > "system.posix_acl_access" and then corrupts memory by not checking the
> > seen_enough state and then trying to shove "trusted.SGI_ACL_FILE" into
> > the buffer as well.
> > 
> > In order to tickle the bug in a user visible way we must have already
> > put a name in the buffer, so we take advantage of the fact that
> > "security.evm" sorts before "system.posix_acl_access" to make sure this
> > happens.
> > 
> > Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> > ---
> >  .gitignore              |    1 
> >  src/Makefile            |    2 -
> >  src/t_attr_corruption.c |  122 +++++++++++++++++++++++++++++++++++++++++++++++
> >  tests/generic/712       |   41 ++++++++++++++++
> >  tests/generic/712.out   |    2 +
> >  tests/generic/group     |    1 
> >  6 files changed, 168 insertions(+), 1 deletion(-)
> >  create mode 100644 src/t_attr_corruption.c
> >  create mode 100755 tests/generic/712
> >  create mode 100644 tests/generic/712.out
> > 
> > diff --git a/.gitignore b/.gitignore
> > index ea1aac8a..0933dc7d 100644
> > --- a/.gitignore
> > +++ b/.gitignore
> > @@ -114,6 +114,7 @@
> >  /src/stat_test
> >  /src/swapon
> >  /src/t_access_root
> > +/src/t_attr_corruption
> >  /src/t_dir_offset
> >  /src/t_dir_offset2
> >  /src/t_dir_type
> > diff --git a/src/Makefile b/src/Makefile
> > index 41826585..ae09eb0a 100644
> > --- a/src/Makefile
> > +++ b/src/Makefile
> > @@ -27,7 +27,7 @@ LINUX_TARGETS = xfsctl bstat t_mtab getdevicesize preallo_rw_pattern_reader \
> >  	renameat2 t_getcwd e4compact test-nextquota punch-alternating \
> >  	attr-list-by-handle-cursor-test listxattr dio-interleaved t_dir_type \
> >  	dio-invalidate-cache stat_test t_encrypted_d_revalidate \
> > -	attr_replace_test swapon mkswap
> > +	attr_replace_test swapon mkswap t_attr_corruption
> >  
> >  SUBDIRS = log-writes perf
> >  
> > diff --git a/src/t_attr_corruption.c b/src/t_attr_corruption.c
> > new file mode 100644
> > index 00000000..1fa5e41f
> > --- /dev/null
> > +++ b/src/t_attr_corruption.c
> > @@ -0,0 +1,122 @@
> > +// SPDX-License-Identifier: GPL-2.0+
> > +/*
> > + * Copyright (C) 2019 Oracle.  All Rights Reserved.
> > + * Author: Darrick J. Wong <darrick.wong@oracle.com>
> > + *
> > + * Test program to tickle a use-after-free bug in xfs.
> > + *
> > + * XFS had a use-after-free bug when xfs_xattr_put_listent runs out of
> > + * listxattr buffer space while trying to store the name
> > + * "system.posix_acl_access" and then corrupts memory by not checking the
> > + * seen_enough state and then trying to shove "trusted.SGI_ACL_FILE" into the
> > + * buffer as well.
> > + *
> > + * In order to tickle the bug in a user visible way we must have already put a
> > + * name in the buffer, so we take advantage of the fact that "security.evm"
> > + * sorts before "system.posix_acl_access" to make sure this happens.
> > + *
> > + * If we trigger the bug, the program will print the garbled string
> > + * "rusted.SGI_ACL_FILE".  If the bug is fixed, the flistxattr call returns
> > + * ERANGE.
> > + */
> > +#include <sys/types.h>
> > +#include <sys/stat.h>
> > +#include <fcntl.h>
> > +#include <stdlib.h>
> > +#include <stdio.h>
> > +#include <string.h>
> > +#include <stdint.h>
> > +#include <unistd.h>
> > +#include <attr/xattr.h>
> > +
> > +void die(const char *msg)
> > +{
> > +	perror(msg);
> > +	exit(1);
> > +}
> > +
> > +struct entry {
> > +	uint16_t	a;
> > +	uint16_t	b;
> > +	uint32_t	c;
> > +};
> > +
> > +struct myacl {
> > +	uint32_t	d;
> > +	struct entry	e[4];
> > +};
> > +
> > +int main(int argc, char *argv[])
> > +{
> > +	struct myacl acl = {
> > +		.d = 2,
> > +		.e = {
> > +			{1, 0, 0},
> > +			{4, 0, 0},
> > +			{0x10, 0, 0},
> > +			{0x20, 0, 0},
> > +		},
> > +	};
> > +	char buf[64];
> > +	ssize_t sz;
> > +	int fd;
> > +	int ret;
> > +
> > +	if (argc > 1) {
> > +		ret = chdir(argv[1]);
> > +		if (ret)
> > +			die(argv[1]);
> > +	}
> > +
> > +	fd = creat("file0", 0644);
> > +	if (fd < 0)
> > +		die("create");
> > +
> > +	ret = fsetxattr(fd, "system.posix_acl_access", &acl, sizeof(acl), 0);
> > +	if (ret)
> > +		die("set posix acl");
> > +
> > +	ret = fsetxattr(fd, "security.evm", buf, 1, 1);
> > +	if (ret)
> > +		die("set evm");
> > +
> > +	sz = flistxattr(fd, buf, 30);
> > +	if (sz < 0)
> > +		die("list attr");
> > +
> > +	printf("%s\n", buf);
> > +
> > +	return 0;
> > +
> > +#if 0
> > +	/* original syzkaller reproducer */
> > +
> > +	syscall(__NR_mmap, 0x20000000, 0x1000000, 3, 0x32, -1, 0);
> > +
> > +	memcpy((void*)0x20000180, "./file0", 8);
> > +	syscall(__NR_creat, 0x20000180, 0);
> > +	memcpy((void*)0x20000000, "./file0", 8);
> > +	memcpy((void*)0x20000040, "system.posix_acl_access", 24);
> > +	*(uint32_t*)0x20000680 = 2;
> > +	*(uint16_t*)0x20000684 = 1;
> > +	*(uint16_t*)0x20000686 = 0;
> > +	*(uint32_t*)0x20000688 = 0;
> > +	*(uint16_t*)0x2000068c = 4;
> > +	*(uint16_t*)0x2000068e = 0;
> > +	*(uint32_t*)0x20000690 = 0;
> > +	*(uint16_t*)0x20000694 = 0x10;
> > +	*(uint16_t*)0x20000696 = 0;
> > +	*(uint32_t*)0x20000698 = 0;
> > +	*(uint16_t*)0x2000069c = 0x20;
> > +	*(uint16_t*)0x2000069e = 0;
> > +	*(uint32_t*)0x200006a0 = 0;
> > +	syscall(__NR_setxattr, 0x20000000, 0x20000040, 0x20000680, 0x24, 0);
> > +	memcpy((void*)0x20000080, "./file0", 8);
> > +	memcpy((void*)0x200000c0, "security.evm", 13);
> > +	memcpy((void*)0x20000100, "\x03\x00\x00\x00\x57", 5);
> > +	syscall(__NR_lsetxattr, 0x20000080, 0x200000c0, 0x20000100, 1, 1);
> > +	memcpy((void*)0x20000300, "./file0", 8);
> > +	syscall(__NR_listxattr, 0x20000300, 0x200002c0, 0x1e);
> > +	return 0;
> > +#endif
> > +}
> > diff --git a/tests/generic/712 b/tests/generic/712
> > new file mode 100755
> > index 00000000..6348a797
> > --- /dev/null
> > +++ b/tests/generic/712
> > @@ -0,0 +1,41 @@
> > +#! /bin/bash
> > +# SPDX-License-Identifier: GPL-2.0+
> > +# Copyright (c) 2019 Oracle, Inc.  All Rights Reserved.
> > +#
> > +# FS QA Test No. 712
> > +#
> > +# Regression test for a bug where XFS corrupts memory if the listxattr buffer
> > +# is a particularly well crafted size on a filesystem that supports posix acls.
> > +#
> > +seq=`basename $0`
> > +seqres=$RESULT_DIR/$seq
> > +echo "QA output created by $seq"
> > +tmp=/tmp/$$
> > +status=1	# failure is the default!
> > +testfile=$TEST_DIR/$seq.txt
> 
> I removed this definition, which is not used in the test.
> 
> > +trap "_cleanup; exit \$status" 0 1 2 3 15
> > +
> > +_cleanup()
> > +{
> > +	cd /
> > +	rm -f $tmp.*
> > +}
> > +
> > +# get standard environment, filters and checks
> > +. ./common/rc
> > +. ./common/attr
> > +
> > +# real QA test starts here
> > +_supported_fs generic
> > +_supported_os Linux
> > +_require_acls
> > +_require_scratch
> 
> I also added
> 
> _require_test_program "t_attr_corruption"

Sounds good, thank you!

> Thanks,
> Eryu
> 
> > +
> > +rm -f $seqres.full
> > +_scratch_mkfs >> $seqres.full 2>&1
> > +_scratch_mount
> > +
> > +src/t_attr_corruption $SCRATCH_MNT
> > +
> > +status=0
> > +exit
> > diff --git a/tests/generic/712.out b/tests/generic/712.out
> > new file mode 100644
> > index 00000000..a2ba09f3
> > --- /dev/null
> > +++ b/tests/generic/712.out
> > @@ -0,0 +1,2 @@
> > +QA output created by 712
> > +list attr: Numerical result out of range
> > diff --git a/tests/generic/group b/tests/generic/group
> > index f56eb475..b3086154 100644
> > --- a/tests/generic/group
> > +++ b/tests/generic/group
> > @@ -529,3 +529,4 @@
> >  524 auto quick
> >  525 auto quick rw
> >  709 auto quick
> > +712 auto quick attr
David Sterba Feb. 20, 2019, 1:29 p.m. UTC | #3
On Wed, Feb 13, 2019 at 12:48:14PM -0800, Darrick J. Wong wrote:
> --- /dev/null
> +++ b/src/t_attr_corruption.c
> @@ -0,0 +1,122 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * Copyright (C) 2019 Oracle.  All Rights Reserved.
> + * Author: Darrick J. Wong <darrick.wong@oracle.com>
> + *
> + * Test program to tickle a use-after-free bug in xfs.
> + *
> + * XFS had a use-after-free bug when xfs_xattr_put_listent runs out of
> + * listxattr buffer space while trying to store the name
> + * "system.posix_acl_access" and then corrupts memory by not checking the
> + * seen_enough state and then trying to shove "trusted.SGI_ACL_FILE" into the
> + * buffer as well.
> + *
> + * In order to tickle the bug in a user visible way we must have already put a
> + * name in the buffer, so we take advantage of the fact that "security.evm"
> + * sorts before "system.posix_acl_access" to make sure this happens.
> + *
> + * If we trigger the bug, the program will print the garbled string
> + * "rusted.SGI_ACL_FILE".  If the bug is fixed, the flistxattr call returns
> + * ERANGE.
> + */
> +#include <sys/types.h>
> +#include <sys/stat.h>
> +#include <fcntl.h>
> +#include <stdlib.h>
> +#include <stdio.h>
> +#include <string.h>
> +#include <stdint.h>
> +#include <unistd.h>
> +#include <attr/xattr.h>

This does not compile on some systems, sys/xattr.h works (it's provided
by glibc) and is also used by other fstests' sources. I'm not sure where
does attr/xattr.h come from, my devel package for libattr provides only
attr/libattr.h.
Holger Hoffstätte Feb. 20, 2019, 2:09 p.m. UTC | #4
On Wed, 20 Feb 2019, David Sterba wrote:

> On Wed, Feb 13, 2019 at 12:48:14PM -0800, Darrick J. Wong wrote:
>> +#include <attr/xattr.h>
>
> This does not compile on some systems, sys/xattr.h works (it's provided
> by glibc) and is also used by other fstests' sources. I'm not sure where
> does attr/xattr.h come from, my devel package for libattr provides only
> attr/libattr.h.

This was removed from attr back in 2015 [1] but apparently caused
many aplications to no longer compile, so some packagers added 
attr/xattr.h back, completing the cycle of confusion.
I just randomly remembered because Gentoo fell into the same sinkhole
and is still climbing out [2].

AFAIK sys/xattr.h is the preferred thing now unless you are building on 
an ancient distro.

cheers
Holger

[1] http://git.savannah.nongnu.org/cgit/attr.git/commit/?id=7921157890d07858d092f4003ca4c6bae9fd2c38
[2] https://bugs.gentoo.org/648864
Darrick J. Wong Feb. 20, 2019, 6:58 p.m. UTC | #5
On Wed, Feb 20, 2019 at 03:09:32PM +0100, Holger Hoffstätte wrote:
> 
> On Wed, 20 Feb 2019, David Sterba wrote:
> 
> > On Wed, Feb 13, 2019 at 12:48:14PM -0800, Darrick J. Wong wrote:
> > > +#include <attr/xattr.h>
> > 
> > This does not compile on some systems, sys/xattr.h works (it's provided
> > by glibc) and is also used by other fstests' sources. I'm not sure where
> > does attr/xattr.h come from, my devel package for libattr provides only
> > attr/libattr.h.
> 
> This was removed from attr back in 2015 [1] but apparently caused
> many aplications to no longer compile, so some packagers added attr/xattr.h
> back, completing the cycle of confusion.
> I just randomly remembered because Gentoo fell into the same sinkhole
> and is still climbing out [2].
> 
> AFAIK sys/xattr.h is the preferred thing now unless you are building on an
> ancient distro.

...an ancient distro like Debian Testing or Ubuntu 18.04, which still
ships libattr 2.4.47, which has manpages telling you to use
attr/xattr.h.

OTOH xfsprogs has been using sys/xattr.h since 2009, so patch coming
soon.

--D

> cheers
> Holger
> 
> [1] http://git.savannah.nongnu.org/cgit/attr.git/commit/?id=7921157890d07858d092f4003ca4c6bae9fd2c38
> [2] https://bugs.gentoo.org/648864
Jeff Mahoney Feb. 25, 2019, 6:57 p.m. UTC | #6
On 2/13/19 3:48 PM, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@oracle.com>
> 
> XFS had a use-after-free bug when xfs_xattr_put_listent runs out of
> listxattr buffer space while trying to store the name
> "system.posix_acl_access" and then corrupts memory by not checking the
> seen_enough state and then trying to shove "trusted.SGI_ACL_FILE" into
> the buffer as well.
> 
> In order to tickle the bug in a user visible way we must have already
> put a name in the buffer, so we take advantage of the fact that
> "security.evm" sorts before "system.posix_acl_access" to make sure this
> happens.
> 
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>

[...]

> +
> +int main(int argc, char *argv[])
> +{
> +	struct myacl acl = {
> +		.d = 2,
> +		.e = {
> +			{1, 0, 0},
> +			{4, 0, 0},
> +			{0x10, 0, 0},
> +			{0x20, 0, 0},
> +		},
> +	};
> +	char buf[64];
> +	ssize_t sz;
> +	int fd;
> +	int ret;
> +
> +	if (argc > 1) {
> +		ret = chdir(argv[1]);
> +		if (ret)
> +			die(argv[1]);
> +	}
> +
> +	fd = creat("file0", 0644);
> +	if (fd < 0)
> +		die("create");
> +
> +	ret = fsetxattr(fd, "system.posix_acl_access", &acl, sizeof(acl), 0);
> +	if (ret)
> +		die("set posix acl");
> +
> +	ret = fsetxattr(fd, "security.evm", buf, 1, 1);
> +	if (ret)
> +		die("set evm");

How is this working on your test system?  The EVM xattr is a formatted
structure and this is passing it an uninitialized buffer.  It *should*
return EPERM and on our test systems it is.

Using security.capability will sort before system.posix_acl_access and
accepts unformatted contents.

-Jeff

> +	sz = flistxattr(fd, buf, 30);
> +	if (sz < 0)
> +		die("list attr");
> +
> +	printf("%s\n", buf);
> +
> +	return 0;
> +
> +#if 0
> +	/* original syzkaller reproducer */
> +
> +	syscall(__NR_mmap, 0x20000000, 0x1000000, 3, 0x32, -1, 0);
> +
> +	memcpy((void*)0x20000180, "./file0", 8);
> +	syscall(__NR_creat, 0x20000180, 0);
> +	memcpy((void*)0x20000000, "./file0", 8);
> +	memcpy((void*)0x20000040, "system.posix_acl_access", 24);
> +	*(uint32_t*)0x20000680 = 2;
> +	*(uint16_t*)0x20000684 = 1;
> +	*(uint16_t*)0x20000686 = 0;
> +	*(uint32_t*)0x20000688 = 0;
> +	*(uint16_t*)0x2000068c = 4;
> +	*(uint16_t*)0x2000068e = 0;
> +	*(uint32_t*)0x20000690 = 0;
> +	*(uint16_t*)0x20000694 = 0x10;
> +	*(uint16_t*)0x20000696 = 0;
> +	*(uint32_t*)0x20000698 = 0;
> +	*(uint16_t*)0x2000069c = 0x20;
> +	*(uint16_t*)0x2000069e = 0;
> +	*(uint32_t*)0x200006a0 = 0;
> +	syscall(__NR_setxattr, 0x20000000, 0x20000040, 0x20000680, 0x24, 0);
> +	memcpy((void*)0x20000080, "./file0", 8);
> +	memcpy((void*)0x200000c0, "security.evm", 13);
> +	memcpy((void*)0x20000100, "\x03\x00\x00\x00\x57", 5);
> +	syscall(__NR_lsetxattr, 0x20000080, 0x200000c0, 0x20000100, 1, 1);
> +	memcpy((void*)0x20000300, "./file0", 8);
> +	syscall(__NR_listxattr, 0x20000300, 0x200002c0, 0x1e);
> +	return 0;
> +#endif
> +}
Darrick J. Wong Feb. 25, 2019, 9 p.m. UTC | #7
On Mon, Feb 25, 2019 at 01:57:51PM -0500, Jeff Mahoney wrote:
> On 2/13/19 3:48 PM, Darrick J. Wong wrote:
> > From: Darrick J. Wong <darrick.wong@oracle.com>
> > 
> > XFS had a use-after-free bug when xfs_xattr_put_listent runs out of
> > listxattr buffer space while trying to store the name
> > "system.posix_acl_access" and then corrupts memory by not checking the
> > seen_enough state and then trying to shove "trusted.SGI_ACL_FILE" into
> > the buffer as well.
> > 
> > In order to tickle the bug in a user visible way we must have already
> > put a name in the buffer, so we take advantage of the fact that
> > "security.evm" sorts before "system.posix_acl_access" to make sure this
> > happens.
> > 
> > Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> 
> [...]
> 
> > +
> > +int main(int argc, char *argv[])
> > +{
> > +	struct myacl acl = {
> > +		.d = 2,
> > +		.e = {
> > +			{1, 0, 0},
> > +			{4, 0, 0},
> > +			{0x10, 0, 0},
> > +			{0x20, 0, 0},
> > +		},
> > +	};
> > +	char buf[64];
> > +	ssize_t sz;
> > +	int fd;
> > +	int ret;
> > +
> > +	if (argc > 1) {
> > +		ret = chdir(argv[1]);
> > +		if (ret)
> > +			die(argv[1]);
> > +	}
> > +
> > +	fd = creat("file0", 0644);
> > +	if (fd < 0)
> > +		die("create");
> > +
> > +	ret = fsetxattr(fd, "system.posix_acl_access", &acl, sizeof(acl), 0);
> > +	if (ret)
> > +		die("set posix acl");
> > +
> > +	ret = fsetxattr(fd, "security.evm", buf, 1, 1);
> > +	if (ret)
> > +		die("set evm");
> 
> How is this working on your test system?

CONFIG_EVM=n, that's how. :(

> The EVM xattr is a formatted structure and this is passing it an
> uninitialized buffer.  It *should* return EPERM and on our test
> systems it is.

Er... what is the structure of the evm attr, anyway?  Does passing in a
single byte 0x03 actually work?

Oh, it's in security/integrity/integrity.h, that's why I couldn't find
it....

enum evm_ima_xattr_type {
	IMA_XATTR_DIGEST = 0x01,
	EVM_XATTR_HMAC,
	EVM_IMA_XATTR_DIGSIG,
	IMA_XATTR_DIGEST_NG,
	EVM_XATTR_PORTABLE_DIGSIG,
	IMA_XATTR_LAST
};

struct evm_ima_xattr_data {
	u8 type;
	u8 digest[SHA1_DIGEST_SIZE];
} __packed;


So I guess we're passing in a xattr_data of type EVM_IMA_XATTR_DIGSIG?
With no actual digest information, which seems suspect to me.

Now I wonder if the VM they used to generate the syzkaller report has
EVM enabled....

(And this is why I hate syzkaller reports, all of the mechanisation I
can't (under)stand and none of the context to help me write a decent
regression test case that actually just friggin works.)

> Using security.capability will sort before system.posix_acl_access and
> accepts unformatted contents.

I'll try that and report back, thank you.  Sorry for the mess.

--D

> -Jeff
> 
> > +	sz = flistxattr(fd, buf, 30);
> > +	if (sz < 0)
> > +		die("list attr");
> > +
> > +	printf("%s\n", buf);
> > +
> > +	return 0;
> > +
> > +#if 0
> > +	/* original syzkaller reproducer */
> > +
> > +	syscall(__NR_mmap, 0x20000000, 0x1000000, 3, 0x32, -1, 0);
> > +
> > +	memcpy((void*)0x20000180, "./file0", 8);
> > +	syscall(__NR_creat, 0x20000180, 0);
> > +	memcpy((void*)0x20000000, "./file0", 8);
> > +	memcpy((void*)0x20000040, "system.posix_acl_access", 24);
> > +	*(uint32_t*)0x20000680 = 2;
> > +	*(uint16_t*)0x20000684 = 1;
> > +	*(uint16_t*)0x20000686 = 0;
> > +	*(uint32_t*)0x20000688 = 0;
> > +	*(uint16_t*)0x2000068c = 4;
> > +	*(uint16_t*)0x2000068e = 0;
> > +	*(uint32_t*)0x20000690 = 0;
> > +	*(uint16_t*)0x20000694 = 0x10;
> > +	*(uint16_t*)0x20000696 = 0;
> > +	*(uint32_t*)0x20000698 = 0;
> > +	*(uint16_t*)0x2000069c = 0x20;
> > +	*(uint16_t*)0x2000069e = 0;
> > +	*(uint32_t*)0x200006a0 = 0;
> > +	syscall(__NR_setxattr, 0x20000000, 0x20000040, 0x20000680, 0x24, 0);
> > +	memcpy((void*)0x20000080, "./file0", 8);
> > +	memcpy((void*)0x200000c0, "security.evm", 13);
> > +	memcpy((void*)0x20000100, "\x03\x00\x00\x00\x57", 5);
> > +	syscall(__NR_lsetxattr, 0x20000080, 0x200000c0, 0x20000100, 1, 1);
> > +	memcpy((void*)0x20000300, "./file0", 8);
> > +	syscall(__NR_listxattr, 0x20000300, 0x200002c0, 0x1e);
> > +	return 0;
> > +#endif
> > +}
> 
> 
> 
> -- 
> Jeff Mahoney
> SUSE Labs

Patch
diff mbox series

diff --git a/.gitignore b/.gitignore
index ea1aac8a..0933dc7d 100644
--- a/.gitignore
+++ b/.gitignore
@@ -114,6 +114,7 @@ 
 /src/stat_test
 /src/swapon
 /src/t_access_root
+/src/t_attr_corruption
 /src/t_dir_offset
 /src/t_dir_offset2
 /src/t_dir_type
diff --git a/src/Makefile b/src/Makefile
index 41826585..ae09eb0a 100644
--- a/src/Makefile
+++ b/src/Makefile
@@ -27,7 +27,7 @@  LINUX_TARGETS = xfsctl bstat t_mtab getdevicesize preallo_rw_pattern_reader \
 	renameat2 t_getcwd e4compact test-nextquota punch-alternating \
 	attr-list-by-handle-cursor-test listxattr dio-interleaved t_dir_type \
 	dio-invalidate-cache stat_test t_encrypted_d_revalidate \
-	attr_replace_test swapon mkswap
+	attr_replace_test swapon mkswap t_attr_corruption
 
 SUBDIRS = log-writes perf
 
diff --git a/src/t_attr_corruption.c b/src/t_attr_corruption.c
new file mode 100644
index 00000000..1fa5e41f
--- /dev/null
+++ b/src/t_attr_corruption.c
@@ -0,0 +1,122 @@ 
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Copyright (C) 2019 Oracle.  All Rights Reserved.
+ * Author: Darrick J. Wong <darrick.wong@oracle.com>
+ *
+ * Test program to tickle a use-after-free bug in xfs.
+ *
+ * XFS had a use-after-free bug when xfs_xattr_put_listent runs out of
+ * listxattr buffer space while trying to store the name
+ * "system.posix_acl_access" and then corrupts memory by not checking the
+ * seen_enough state and then trying to shove "trusted.SGI_ACL_FILE" into the
+ * buffer as well.
+ *
+ * In order to tickle the bug in a user visible way we must have already put a
+ * name in the buffer, so we take advantage of the fact that "security.evm"
+ * sorts before "system.posix_acl_access" to make sure this happens.
+ *
+ * If we trigger the bug, the program will print the garbled string
+ * "rusted.SGI_ACL_FILE".  If the bug is fixed, the flistxattr call returns
+ * ERANGE.
+ */
+#include <sys/types.h>
+#include <sys/stat.h>
+#include <fcntl.h>
+#include <stdlib.h>
+#include <stdio.h>
+#include <string.h>
+#include <stdint.h>
+#include <unistd.h>
+#include <attr/xattr.h>
+
+void die(const char *msg)
+{
+	perror(msg);
+	exit(1);
+}
+
+struct entry {
+	uint16_t	a;
+	uint16_t	b;
+	uint32_t	c;
+};
+
+struct myacl {
+	uint32_t	d;
+	struct entry	e[4];
+};
+
+int main(int argc, char *argv[])
+{
+	struct myacl acl = {
+		.d = 2,
+		.e = {
+			{1, 0, 0},
+			{4, 0, 0},
+			{0x10, 0, 0},
+			{0x20, 0, 0},
+		},
+	};
+	char buf[64];
+	ssize_t sz;
+	int fd;
+	int ret;
+
+	if (argc > 1) {
+		ret = chdir(argv[1]);
+		if (ret)
+			die(argv[1]);
+	}
+
+	fd = creat("file0", 0644);
+	if (fd < 0)
+		die("create");
+
+	ret = fsetxattr(fd, "system.posix_acl_access", &acl, sizeof(acl), 0);
+	if (ret)
+		die("set posix acl");
+
+	ret = fsetxattr(fd, "security.evm", buf, 1, 1);
+	if (ret)
+		die("set evm");
+
+	sz = flistxattr(fd, buf, 30);
+	if (sz < 0)
+		die("list attr");
+
+	printf("%s\n", buf);
+
+	return 0;
+
+#if 0
+	/* original syzkaller reproducer */
+
+	syscall(__NR_mmap, 0x20000000, 0x1000000, 3, 0x32, -1, 0);
+
+	memcpy((void*)0x20000180, "./file0", 8);
+	syscall(__NR_creat, 0x20000180, 0);
+	memcpy((void*)0x20000000, "./file0", 8);
+	memcpy((void*)0x20000040, "system.posix_acl_access", 24);
+	*(uint32_t*)0x20000680 = 2;
+	*(uint16_t*)0x20000684 = 1;
+	*(uint16_t*)0x20000686 = 0;
+	*(uint32_t*)0x20000688 = 0;
+	*(uint16_t*)0x2000068c = 4;
+	*(uint16_t*)0x2000068e = 0;
+	*(uint32_t*)0x20000690 = 0;
+	*(uint16_t*)0x20000694 = 0x10;
+	*(uint16_t*)0x20000696 = 0;
+	*(uint32_t*)0x20000698 = 0;
+	*(uint16_t*)0x2000069c = 0x20;
+	*(uint16_t*)0x2000069e = 0;
+	*(uint32_t*)0x200006a0 = 0;
+	syscall(__NR_setxattr, 0x20000000, 0x20000040, 0x20000680, 0x24, 0);
+	memcpy((void*)0x20000080, "./file0", 8);
+	memcpy((void*)0x200000c0, "security.evm", 13);
+	memcpy((void*)0x20000100, "\x03\x00\x00\x00\x57", 5);
+	syscall(__NR_lsetxattr, 0x20000080, 0x200000c0, 0x20000100, 1, 1);
+	memcpy((void*)0x20000300, "./file0", 8);
+	syscall(__NR_listxattr, 0x20000300, 0x200002c0, 0x1e);
+	return 0;
+#endif
+}
diff --git a/tests/generic/712 b/tests/generic/712
new file mode 100755
index 00000000..6348a797
--- /dev/null
+++ b/tests/generic/712
@@ -0,0 +1,41 @@ 
+#! /bin/bash
+# SPDX-License-Identifier: GPL-2.0+
+# Copyright (c) 2019 Oracle, Inc.  All Rights Reserved.
+#
+# FS QA Test No. 712
+#
+# Regression test for a bug where XFS corrupts memory if the listxattr buffer
+# is a particularly well crafted size on a filesystem that supports posix acls.
+#
+seq=`basename $0`
+seqres=$RESULT_DIR/$seq
+echo "QA output created by $seq"
+tmp=/tmp/$$
+status=1	# failure is the default!
+testfile=$TEST_DIR/$seq.txt
+trap "_cleanup; exit \$status" 0 1 2 3 15
+
+_cleanup()
+{
+	cd /
+	rm -f $tmp.*
+}
+
+# get standard environment, filters and checks
+. ./common/rc
+. ./common/attr
+
+# real QA test starts here
+_supported_fs generic
+_supported_os Linux
+_require_acls
+_require_scratch
+
+rm -f $seqres.full
+_scratch_mkfs >> $seqres.full 2>&1
+_scratch_mount
+
+src/t_attr_corruption $SCRATCH_MNT
+
+status=0
+exit
diff --git a/tests/generic/712.out b/tests/generic/712.out
new file mode 100644
index 00000000..a2ba09f3
--- /dev/null
+++ b/tests/generic/712.out
@@ -0,0 +1,2 @@ 
+QA output created by 712
+list attr: Numerical result out of range
diff --git a/tests/generic/group b/tests/generic/group
index f56eb475..b3086154 100644
--- a/tests/generic/group
+++ b/tests/generic/group
@@ -529,3 +529,4 @@ 
 524 auto quick
 525 auto quick rw
 709 auto quick
+712 auto quick attr