Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932650AbcJULQF (ORCPT ); Fri, 21 Oct 2016 07:16:05 -0400 Received: from mx1.redhat.com ([209.132.183.28]:37496 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932694AbcJULQC (ORCPT ); Fri, 21 Oct 2016 07:16:02 -0400 From: Vitaly Kuznetsov To: David Miller Cc: sthemmin@microsoft.com, netdev@vger.kernel.org, devel@linuxdriverproject.org, linux-kernel@vger.kernel.org, kys@microsoft.com, haiyangz@microsoft.com Subject: Re: [PATCH net-next] hv_netvsc: fix a race between netvsc_send() and netvsc_init_buf() References: <1476885181-3456-1-git-send-email-vkuznets@redhat.com> <8737jr1k07.fsf@vitty.brq.redhat.com> <20161020.140300.122827393647670926.davem@davemloft.net> Date: Fri, 21 Oct 2016 13:15:53 +0200 In-Reply-To: <20161020.140300.122827393647670926.davem@davemloft.net> (David Miller's message of "Thu, 20 Oct 2016 14:03:00 -0400 (EDT)") Message-ID: <87d1iuymty.fsf@vitty.brq.redhat.com> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/25.1 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.39]); Fri, 21 Oct 2016 11:15:57 +0000 (UTC) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 1699 Lines: 44 David Miller writes: > From: Vitaly Kuznetsov > Date: Thu, 20 Oct 2016 10:51:04 +0200 > >> Stephen Hemminger writes: >> >>> Do we need ACCESS_ONCE() here to avoid check/use issues? >>> >> >> I think we don't: this is the only place in the function where we read >> the variable so we'll get normal read. We're not trying to syncronize >> with netvsc_init_buf() as that would require locking, if we read stale >> NULL value after it was already updated on a different CPU we're fine, >> we'll just return -EAGAIN. > > The concern is if we race with netvsc_destroy_buf() and this pointer > becomes NULL after the test you are adding. Thanks, this is interesting. We may reach to netvsc_destroy_buf() by 3 pathes: 1) cleanup path in netvsc_init_buf(). It is never called with send_section_map being not NULL so it seems we're safe. 2) from netvsc_remove() when the device is being removed. As far as I understand we can't be on the transmit path after we call unregister_netdev() so we're safe. 3) from netvsc_change_mtu() and netvsc_set_channels(). These pathes are specific to netvsc driver as basically we need to remove the device and add it back to change mtu/number of channels. The underligning 'struct net_device' stays but the rest is being removed and added back. On both pathes we first call netvsc_close() which does netif_tx_disable() and as far as I understand (I may be wrong) this guarantees that we won't be in netvsc_send(). So *I think* that we can't ever free send_section_map while in netvsc_send() and we can't even get to netvsc_send() after it is freed but I may have missed something. -- Vitaly