From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-4.0 required=3.0 tests=HEADER_FROM_DIFFERENT_DOMAINS, MAILING_LIST_MULTI,SIGNED_OFF_BY,SPF_PASS,URIBL_BLOCKED autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 01CF4C43381 for ; Thu, 21 Feb 2019 04:07:10 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id C276C2147A for ; Thu, 21 Feb 2019 04:07:10 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1726415AbfBUEHJ (ORCPT ); Wed, 20 Feb 2019 23:07:09 -0500 Received: from shards.monkeyblade.net ([23.128.96.9]:60034 "EHLO shards.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726113AbfBUEHJ (ORCPT ); Wed, 20 Feb 2019 23:07:09 -0500 Received: from localhost (unknown [IPv6:2601:601:9f80:35cd::bf5]) (using TLSv1 with cipher AES256-SHA (256/256 bits)) (Client did not present a certificate) (Authenticated sender: davem-davemloft) by shards.monkeyblade.net (Postfix) with ESMTPSA id 0726D12DE57F5; Wed, 20 Feb 2019 20:07:08 -0800 (PST) Date: Wed, 20 Feb 2019 20:07:08 -0800 (PST) Message-Id: <20190220.200708.417702200968579325.davem@davemloft.net> To: viro@zeniv.linux.org.uk Cc: netdev@vger.kernel.org Subject: Re: [PATCH][unix] missing barriers in some of unix_sock ->addr and ->path accesses From: David Miller In-Reply-To: <20190215200934.GM2217@ZenIV.linux.org.uk> References: <20190215200934.GM2217@ZenIV.linux.org.uk> X-Mailer: Mew version 6.8 on Emacs 26.1 Mime-Version: 1.0 Content-Type: Text/Plain; charset=us-ascii Content-Transfer-Encoding: 7bit X-Greylist: Sender succeeded SMTP AUTH, not delayed by milter-greylist-4.5.12 (shards.monkeyblade.net [149.20.54.216]); Wed, 20 Feb 2019 20:07:09 -0800 (PST) Sender: netdev-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org From: Al Viro Date: Fri, 15 Feb 2019 20:09:35 +0000 > Several u->addr and u->path users are not holding any locks in > common with unix_bind(). unix_state_lock() is useless for those > purposes. > > u->addr is assign-once and *(u->addr) is fully set up by the time > we set u->addr (all under unix_table_lock). u->path is also > set in the same critical area, also before setting u->addr, and > any unix_sock with ->path filled will have non-NULL ->addr. > > So setting ->addr with smp_store_release() is all we need for those > "lockless" users - just have them fetch ->addr with smp_load_acquire() > and don't even bother looking at ->path if they see NULL ->addr. > > Users of ->addr and ->path fall into several classes now: > 1) ones that do smp_load_acquire(u->addr) and access *(u->addr) > and u->path only if smp_load_acquire() has returned non-NULL. > 2) places holding unix_table_lock. These are guaranteed that > *(u->addr) is seen fully initialized. If unix_sock is in one of the > "bound" chains, so's ->path. > 3) unix_sock_destructor() using ->addr is safe. All places > that set u->addr are guaranteed to have seen all stores *(u->addr) > while holding a reference to u and unix_sock_destructor() is called > when (atomic) refcount hits zero. > 4) unix_release_sock() using ->path is safe. unix_bind() > is serialized wrt unix_release() (normally - by struct file > refcount), and for the instances that had ->path set by unix_bind() > unix_release_sock() comes from unix_release(), so they are fine. > Instances that had it set in unix_stream_connect() either end up > attached to a socket (in unix_accept()), in which case the call > chain to unix_release_sock() and serialization are the same as in > the previous case, or they never get accept'ed and unix_release_sock() > is called when the listener is shut down and its queue gets purged. > In that case the listener's queue lock provides the barriers needed - > unix_stream_connect() shoves our unix_sock into listener's queue > under that lock right after having set ->path and eventual > unix_release_sock() caller picks them from that queue under the > same lock right before calling unix_release_sock(). > 5) unix_find_other() use of ->path is pointless, but safe - > it happens with successful lookup by (abstract) name, so ->path.dentry > is guaranteed to be NULL there. > > earlier-variant-reviewed-by: "Paul E. McKenney" > Signed-off-by: Al Viro Applied and queued up for -stable, thanks Al.