Re: [PATCH 2/2] t_attr_corruption: fix this yet again

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 2/25/19 9:35 PM, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@xxxxxxxxxx>
> 
> Jeff Moyer pointed out that 'security.evm' actually has an expected

Mahoney :)

> value format, which breaks the test if EVM is enabled.  It turns out
> that the 'security.evm' setxattr call in the original syzkaller report
> was a total red herring, as this bug can be reproduced without it.
> 
> Fix the test case to do the minimum amount of work needed to reproduce
> the corruption.
> 
> Signed-off-by: Darrick J. Wong <darrick.wong@xxxxxxxxxx>

Otherwise, works for me in the case that failed for me previously.

Thanks,

-Jeff

> ---
>  src/t_attr_corruption.c |   28 ++++++++++++++--------------
>  1 file changed, 14 insertions(+), 14 deletions(-)
> 
> 
> diff --git a/src/t_attr_corruption.c b/src/t_attr_corruption.c
> index f26611f9..9101024e 100644
> --- a/src/t_attr_corruption.c
> +++ b/src/t_attr_corruption.c
> @@ -3,17 +3,21 @@
>   * Copyright (C) 2019 Oracle.  All Rights Reserved.
>   * Author: Darrick J. Wong <darrick.wong@xxxxxxxxxx>
>   *
> - * Test program to tickle a use-after-free bug in xfs.
> + * XFS had a memory corruption bug in its handling of the POSIX ACL attribute
> + * names during a listxattr call.
>   *
> - * 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.
> + * On IRIX, file ACLs were stored under the name "trusted.SGI_ACL_FILE",
> + * whereas on Linux the name is "system.posix_acl_access".  In order to
> + * maintain compatibility with old filesystems, XFS internally continues to
> + * use the old SGI_ACL_FILE name on disk and remap the new name whenever it
> + * sees it.
>   *
> - * 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.
> + * In order to make this magic happen, XFS' listxattr implementation will emit
> + * first the Linux name and then the on-disk name.  Unfortunately, it doesn't
> + * correctly check the buffer length, so if the buffer is large enough to fit
> + * the on-disk name but not large enough to fit the Linux name, we screw up
> + * the buffer position accounting while trying to emit the Linux name and then
> + * corrupt memory when we try to emit the on-disk name.
>   *
>   * 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
> @@ -76,11 +80,7 @@ int main(int argc, char *argv[])
>  	if (ret)
>  		die("set posix acl");
>  
> -	ret = fsetxattr(fd, "security.evm", buf, 1, 1);
> -	if (ret)
> -		die("set evm");
> -
> -	sz = flistxattr(fd, buf, 30);
> +	sz = flistxattr(fd, buf, 20);
>  	if (sz < 0)
>  		die("list attr");
>  
> 
> 


-- 
Jeff Mahoney
SUSE Labs



[Index of Archives]     [XFS Filesystem Development (older mail)]     [Linux Filesystem Development]     [Linux Audio Users]     [Yosemite Trails]     [Linux Kernel]     [Linux RAID]     [Linux SCSI]


  Powered by Linux