linux-kernel - Re: [PATCH v8 02/10] kernfs: make ->attr.open RCU protected.

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]

Message-ID: <Ymg76nQOOv7hwtc1@slm.duckdns.org>
Date:   Tue, 26 Apr 2022 08:37:30 -1000
From:   Tejun Heo <tj@...nel.org>
To:     Imran Khan <imran.f.khan@...cle.com>
Cc:     viro@...iv.linux.org.uk, gregkh@...uxfoundation.org,
        ebiederm@...ssion.com, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v8 02/10] kernfs: make ->attr.open RCU protected.

On Tue, Apr 26, 2022 at 11:54:36AM +1000, Imran Khan wrote:
> >> @@ -912,14 +920,13 @@ void kernfs_notify(struct kernfs_node *kn)
> >>  		return;
> >>  
> >>  	/* kick poll immediately */
> >> -	spin_lock_irqsave(&kernfs_open_node_lock, flags);
> >> -	on = kn->attr.open;
> >> +	rcu_read_lock();
> >> +	on = rcu_dereference(kn->attr.open);
> >>  	if (on) {
> >>  		atomic_inc(&on->event);
> >>  		wake_up_interruptible(&on->poll);
> >>  	}
> >> -	spin_unlock_irqrestore(&kernfs_open_node_lock, flags);
> >> -
> >> +	rcu_read_unlock();
> > 
> > An explanation of why this is safe in terms of event ordering would be great
> > here.
> > 
> 
> This is safe because here we don't need to refcnt ->on in this case. If
> writer (kernfs_put_open_node) has already made ->attr.open NULL we will
> bail out. If kernfs_notify got an old ->attr.open we can still safely
> process the event, even if kernfs_put_open_node updates ->attr.open to
> NULL in parallel.
> In both the cases the behaviour/order will be same as earlier code that
> used kernfs_open_node_lock.
> Please let me know if this answers your query or if something is still
> missing.

I was more wondering whether if there's someone waiting for an event which
should arrive considering the sequence of events, whether the lockless code
would have the same ordering properties. I think it does given that the
event sequence can only be defined in memory visibility terms anyway.

Thanks.

-- 
tejun