linux-kernel - Re: [RESEND PATCH v7 1/8] kernfs: Introduce interface to access global kernfs_open_file

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:   Tue, 5 Apr 2022 15:36:03 +1000
From:   Imran Khan <imran.f.khan@...cle.com>
To:     Al Viro <viro@...iv.linux.org.uk>
Cc:     tj@...nel.org, gregkh@...uxfoundation.org,
        akpm@...ux-foundation.org, linux-kernel@...r.kernel.org
Subject: Re: [RESEND PATCH v7 1/8] kernfs: Introduce interface to access
 global kernfs_open_file_mutex.

Hello Al,

On 18/3/22 8:34 am, Al Viro wrote:
> On Thu, Mar 17, 2022 at 06:26:05PM +1100, Imran Khan wrote:
> 
>> @@ -570,9 +571,10 @@ static void kernfs_put_open_node(struct kernfs_node *kn,
>>  				 struct kernfs_open_file *of)
[...]

> As the matter of fact, we can do even better - make freeing
> that thing rcu-delayed, use rcu_assign_pointer() for stores,
> rcu_dereference() for loads and have kernfs_notify() do
> 	rcu_read_lock();
> 	on = rcu_dereference(kn->attr.open);
> 	if (on) {
> 		atomic_inc(&on->event);
> 		wake_up_interruptible(&on->poll);
> 	}
> 	rcu_read_unlock();
> and kernfs_open_node_lock becomes useless - all places that
> grab it are under kernfs_open_file_mutex.

There are some issues in freeing ->attr.open under RCU callback. There
are some users of ->attr.open that can block and hence can't operate
under rcu_read_lock. For example in kernfs_drain_open_files we are
traversing list of open files corresponding to ->attr.open and unmapping
those as well. The unmapping operation can block in i_mmap_lock_write.
So even after removing refcnt we will still need kernfs_open_node_lock.

Thanks,
-- Imran