| Message ID | 20190213204814.GB6477@magnolia (mailing list archive) |
|---|---|
| State | Accepted |
| Headers | show |
| Series | fstests: fixes and new tests | expand |
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
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
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.
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
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
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 > +}
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
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