linux-kernel - Re: [PATCH] staging: lustre: llite: drop acl from cache

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date:	Wed, 25 May 2016 00:08:33 +0200
From:	Andreas Gruenbacher <agruenba@...hat.com>
To:	James Simmons <jsimmons@...radead.org>
Cc:	Andreas Grünbacher <andreas.gruenbacher@...il.com>,
	Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
	devel@...verdev.osuosl.org,
	Andreas Dilger <andreas.dilger@...el.com>,
	Oleg Drokin <oleg.drokin@...el.com>,
	Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
	Lustre Development List <lustre-devel@...ts.lustre.org>
Subject: Re: [PATCH] staging: lustre: llite: drop acl from cache

On Tue, May 24, 2016 at 10:38 PM, James Simmons <jsimmons@...radead.org> wrote:
>> 2016-05-24 2:35 GMT+02:00 James Simmons <jsimmons@...radead.org>:
>> > Commit b8a7a3a6 change get_acl() for posix xattr to always cache
>> > the ACL which increases the reference count. That reference count
>> > can be reduced by have ll_get_acl() call forget_cached_acl() which
>> > it wasn't. When an inode gets deleted by Lustre the POSIX ACL
>> > reference count is tested to ensure its 1 and if not produces an error.
>>
>> Lustre shouldn't assume that the VFS immediately drops the reference
>> it is passed. Please remove that check as well.
>
> The piece of code in question from ll_delete_inode() is
>
> #ifdef CONFIG_FS_POSIX_ACL
>         else if (lli->lli_posix_acl) {
>                 LASSERT(atomic_read(&lli->lli_posix_acl->a_refcount) ==
> 1);
>                 LASSERT(!lli->lli_remote_perms);
>                 posix_acl_release(lli->lli_posix_acl);
>                 lli->lli_posix_acl = NULL;
>         }
> #endif
>
> So we want to prevent a leak should I do a
>
> while (atomic_read(&lli->lli_posix_acl->a_refcount))
>         posix_acl_release(lli->lli_posix_acl);
> lli->lli_posix_acl = NULL;
>
> Or does the VFS do this cleanup for us?

This conversation is unreal. Just remove the misguided assert and
you're good. After that, please have someone explain basic reference
counting to you.

Thanks,
Andreas