linux-kernel - Re: [PATCH v6 1/4] xattr: extract simple

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] [thread-next>] [day] [month] [year] [list]

Date:	Wed, 22 Aug 2012 16:55:57 -0400
From:	Aristeu Rozanski <aris@...hat.com>
To:	Hugh Dickins <hughd@...gle.com>
Cc:	Tejun Heo <tj@...nel.org>, linux-kernel@...r.kernel.org,
	cgroups@...r.kernel.org, Li Zefan <lizefan@...wei.com>,
	Hillf Danton <dhillf@...il.com>,
	Lennart Poettering <lpoetter@...hat.com>
Subject: Re: [PATCH v6 1/4] xattr: extract simple_xattr code from tmpfs

On Wed, Aug 22, 2012 at 01:25:06PM -0700, Hugh Dickins wrote:
> > > Probably more important would be to remove spin_lock() and spin_unlock()
> > > (and INIT_LIST_HEAD) from simple_xattrs_free() - those are unnecessary
> > > in shmem_evict_inode(), and wouldn't they be unnecessary whenever
> > > simple_xattrs_free() gets called?
> > 
> > Removing INIT_LIST_HEAD() it's possible by actually unlinking each xattr
> > inside the loop before freeing them. still, it'll have to check if the list is
> > empty or not, which might end up being the same?
> > 
> > About the locking, I'm not sure, I'm investigating it.
> 
> I think we have a misunderstanding.
> 
> INIT_LIST_HEAD() is not expensive, I just meant to remove it because
> I thought it unnecessary by that point.

ah, I see.

> Do you envisage anywhere that would call simple_xattrs_free() except
> a filesystem's evict_inode()?

cgroup does it differently and it's called in d_iput() path (see cgroup_diput),
because it needs to selectively remove files upon remount.

> By that point, the inode is on its way out of the system: nothing
> much (yes, I am being a bit vague there ;) can get to it any more,
> there's no need to reinitialize the list head and there's no need for
> locking, because nothing else can be playing with those xattrs now.

I agree with you. That's why I'm looking into it because I'm pretty sure
I removed it at some point in the past and decided to put it back after
investigating the easily reproducible oops. Sadly I managed to forget
the analisys I did at the time.

-- 
Aristeu

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/