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 Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 6C723C433F5 for ; Fri, 15 Apr 2022 15:49:55 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1355662AbiDOPwU (ORCPT ); Fri, 15 Apr 2022 11:52:20 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:33362 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1344499AbiDOPwL (ORCPT ); Fri, 15 Apr 2022 11:52:11 -0400 Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) by lindbergh.monkeyblade.net (Postfix) with ESMTP id 6423E17A87 for ; Fri, 15 Apr 2022 08:49:42 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1650037781; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=xuws84Z0jp4NihhTFNZTBNeED6UNObMvI7eL6H7Gs80=; b=Q62qY+jFPSM29dBPtHmf4OiQev367SUIPEqcK3De4JL9ESVJuCXrzTCS2/AYfKqxWn0wld VYs/w9O6TjciqZK8gf6WxlxNv8UvPCBXcDTPNylIPw0g1VY429cvw03IwDuP7Q+5gFYfaz biuhNKlTeHk5HPqLM9/NhtbZSmWZ58M= Received: from mimecast-mx02.redhat.com (mimecast-mx02.redhat.com [66.187.233.88]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id us-mta-203-IMKEHtUKOHa8JFhxqrqjmA-1; Fri, 15 Apr 2022 11:49:36 -0400 X-MC-Unique: IMKEHtUKOHa8JFhxqrqjmA-1 Received: from smtp.corp.redhat.com (int-mx02.intmail.prod.int.rdu2.redhat.com [10.11.54.2]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx02.redhat.com (Postfix) with ESMTPS id 965C51014A61; Fri, 15 Apr 2022 15:49:35 +0000 (UTC) Received: from ceranb (unknown [10.40.194.169]) by smtp.corp.redhat.com (Postfix) with ESMTP id A0114404E4B0; Fri, 15 Apr 2022 15:49:33 +0000 (UTC) Date: Fri, 15 Apr 2022 17:49:32 +0200 From: Ivan Vecera To: Michal Schmidt Cc: netdev@vger.kernel.org, Petr Oros , Jesse Brandeburg , Tony Nguyen , "David S. Miller" , Jakub Kicinski , Paolo Abeni , Shiraz Saleem , Dave Ertman , "moderated list:INTEL ETHERNET DRIVERS" , open list Subject: Re: [PATCH net] ice: Fix race during aux device (un)plugging Message-ID: <20220415174932.6c85d5ab@ceranb> In-Reply-To: References: <20220414163907.1456925-1-ivecera@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit X-Scanned-By: MIMEDefang 2.84 on 10.11.54.2 Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Fri, 15 Apr 2022 13:12:03 +0200 Michal Schmidt wrote: > On Thu, Apr 14, 2022 at 6:39 PM Ivan Vecera wrote: > > > Function ice_plug_aux_dev() assigns pf->adev field too early prior > > aux device initialization and on other side ice_unplug_aux_dev() > > starts aux device deinit and at the end assigns NULL to pf->adev. > > This is wrong and can causes a crash when ice_send_event_to_aux() > > call occurs during these operations because that function depends > > on non-NULL value of pf->adev and does not assume that aux device > > is half-initialized or half-destroyed. > > > > Modify affected functions so pf->adev field is set after aux device > > init and prior aux device destroy. > > > [...] > > > @@ -320,12 +319,14 @@ int ice_plug_aux_dev(struct ice_pf *pf) > > */ > > void ice_unplug_aux_dev(struct ice_pf *pf) > > { > > - if (!pf->adev) > > + struct auxiliary_device *adev = pf->adev; > > + > > + if (!adev) > > return; > > > > - auxiliary_device_delete(pf->adev); > > - auxiliary_device_uninit(pf->adev); > > pf->adev = NULL; > > + auxiliary_device_delete(adev); > > + auxiliary_device_uninit(adev); > > } > > > > Hi Ivan, > What prevents ice_unplug_aux_dev() from running immediately after > ice_send_event_to_aux() gets past its "if (!pf->adev)" test ? > Michal ice_send_event_to_aux() takes aux device lock. ice_unplug_aux_dev() calls auxiliary_device_delete() that calls device_del(). device_del() takes device_lock() prior kill_device(). So if ice_send_event_to_aux() is in progress then device_del() waits for its completion. Thanks, Ivan