linux-kernel - Re: [RESEND PATCH] kernfs: Avoid re-adding kernfs_node into kernfs_notify

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: <Yr9U1q0BBinCgyrT@mtj.duckdns.org>
Date:   Sat, 2 Jul 2022 05:11:02 +0900
From:   Tejun Heo <tj@...nel.org>
To:     Imran Khan <imran.f.khan@...cle.com>
Cc:     gregkh@...uxfoundation.org, viro@...iv.linux.org.uk,
        m.szyprowski@...sung.com, nathan@...nel.org, michael@...le.cc,
        robh@...nel.org, linux-serial@...r.kernel.org,
        linux-kernel@...r.kernel.org, guillaume.tucker@...labora.com,
        pmladek@...e.com
Subject: Re: [RESEND PATCH] kernfs: Avoid re-adding kernfs_node into
 kernfs_notify_list.

Hello,

On Sat, Jul 02, 2022 at 01:46:04AM +1000, Imran Khan wrote:
> @@ -992,9 +993,11 @@ void kernfs_notify(struct kernfs_node *kn)
>  	rcu_read_unlock();
>  
>  	/* schedule work to kick fsnotify */
> -	kernfs_get(kn);
> -	llist_add(&kn->attr.notify_next, &kernfs_notify_list);
> -	schedule_work(&kernfs_notify_work);
> +	if (kn->attr.notify_next.next != NULL) {
> +		kernfs_get(kn);
> +		llist_add(&kn->attr.notify_next, &kernfs_notify_list);
> +		schedule_work(&kernfs_notify_work);
> +	}

Aren't you just narrowing the race window here? What prevents two
threads simultaneously testing for non NULL and then entering the
addition block together?

Looked at the llist code and it doesn't support multiple producers
trying to add the same node, unfortunately, so I'm not sure llist is
gonna work here. For now, the right thing to do prolly is reverting
it.

Al, is there something I'm missing about llist?

Thanks.

-- 
tejun