Re: [PATCH v2] posix_acl: fix memleak when set posix acl.

[Al Viro <viro@zeniv.linux.org.uk>]

On Tue, Nov 26, 2019 at 09:38:09PM +0800, Zhang Xiaoxu wrote:
> When set posix acl, it maybe call posix_acl_update_mode in some
> filesystem, eg. ext4. It may set acl to NULL, so, we can't free
> the acl which allocated in posix_acl_xattr_set.
> 
> Use an temp value to store the acl address for posix_acl_release.

Huh?

>  {
> -	struct posix_acl *acl = NULL;
> +	struct posix_acl *acl = NULL, *p = NULL;
>  	int ret;
>  
>  	if (value) {
> @@ -890,8 +890,15 @@ posix_acl_xattr_set(const struct xattr_handler *handler,
>  		if (IS_ERR(acl))
>  			return PTR_ERR(acl);
>  	}
> +
> +	/*
> +	 * when call set_posix_acl, posix_acl_update_mode may set acl
> +	 * to NULL,use temporary variables p for posix_acl_release.
> +	 */
> +	p = acl;
>  	ret = set_posix_acl(inode, handler->flags, acl);
> -	posix_acl_release(acl);
> +
> +	posix_acl_release(p);

	How could set_posix_acl() possibly set a local variable of
posix_acl_xattr_set() to NULL or to anything else, for that matter?
That makes no sense.  C passes arguments by value; formal parameters
behave as local variables in the called function, initialized by
the values passed by caller.  Modifying those inside the called
function is perfectly valid, same as for any local variable.  And
it does _not_ modify anything in the caller's scope.

	Do yourself a favour, grab a textbook on C (or the actual
standard, if you are up for that - e.g. at
http://www.open-std.org/jtc1/sc22/wg14/www/docs/n1256.pdf) and
read it through.  That'll save you a lot of frustration trying to
guess what some construct is doing.