From nobody Mon May 25 05:54:39 2026 Received: from mail-wr1-f45.google.com (mail-wr1-f45.google.com [209.85.221.45]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id C32AF3D47DD for ; Sun, 17 May 2026 22:21:27 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.221.45 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1779056490; cv=none; b=cxPDc22iYGWHTJ0wFF1u1R/UO27GvDI54MLXiwV6yte2jJ62FgHR1T1xoLcIkWPNsdMosKFmtgmzjCeRMHBsb4d2ygot1uf0+/xT7EuNTIiu2VnSHLMbRjC97WHtJkxeYgoqDmwYCeQxUvOEBLEgGsN92CKuRtNoKxo3PWOm5TI= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1779056490; c=relaxed/simple; bh=Nkdh/o+NdvEDXczKo7JSi5D+SMLbhUf2r08VjQQK1u8=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=ZSzbkuzITb0hR7sOdus4GNGD3V2FYnhzeK3UxQSf3/VM/Du4e7nKtzQIrxVzhkdBWKnDxVUhUnDOWIPgl8Iv5/lVtG7qiZxMrwm6FDNSjrSOzE2vQKu1ca9KFDdlAxMu5fo0D2WBw0FL3J8GRkndX1NcXUhPs+EGfUQZVvrxRxk= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com; spf=pass smtp.mailfrom=gmail.com; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b=dJGnA0hz; arc=none smtp.client-ip=209.85.221.45 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=gmail.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b="dJGnA0hz" Received: by mail-wr1-f45.google.com with SMTP id ffacd0b85a97d-449de065cb3so1510967f8f.2 for ; Sun, 17 May 2026 15:21:27 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20251104; t=1779056486; x=1779661286; darn=vger.kernel.org; h=user-agent:in-reply-to:content-disposition:mime-version:references :message-id:subject:cc:to:from:date:from:to:cc:subject:date :message-id:reply-to; bh=jRXKAgR6mEIDdOY7jCBs+jQiL9+n0WTtYZp0RJAX6B0=; b=dJGnA0hzcwA21mytuA1t2RaR1bFa9b2pQBy4dD3hIWomjLekUFiTIZ9PWC14nGWxCc CaU7mkpSL0XJfABX+H+XdmVqIC7wOte88Ydy9t6VSzQrih2gf1A+Pfkj73/+UXcJ5keT c9xhp2RyG9C9+p5TqCdtc2bO/tMNSG2AFDDMTMFTeq5mtKT2M3bgQpe3TVicipeNn6NJ bkrfGBmB8YWqojaObo0yg4Q3GKC8pdSXtGTcpkhYDBpWy4Ur9ttRrBVF01zcitDYMfpI WBz2kqvMSl0Irp87OL2/py18HXrE+7B8HvMgzeo6PbdB729d94+l18ZPqIcd7aLmcfl6 PJ3A== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20251104; t=1779056486; x=1779661286; h=user-agent:in-reply-to:content-disposition:mime-version:references :message-id:subject:cc:to:from:date:x-gm-gg:x-gm-message-state:from :to:cc:subject:date:message-id:reply-to; bh=jRXKAgR6mEIDdOY7jCBs+jQiL9+n0WTtYZp0RJAX6B0=; b=niB4Ok3BWGaFa3TfjoF6wusE1mIPa+5c4pWaIuS6ROOANG8C8gSiQ1PBqaar8khsvW 7lvzOO5oxAprdump6B3ZHkRdCafwhPo7HxgSqkTH4lVcfl80vrm+u0xXGD13JvD/Orln s+t3DrsNsaLeaF4lwBR2eEGoOOPcbJkz6FryD1atKunQ/M1as5OD7xHEBfNk4SFAOFOM EdN7+Fl6rZyIMBsnauEL5hYPqrd+2FoCaKNGY/4dNxVKMInWXxdY68jj3mOJzsVmPcUt vIvZlmk+2n6AOho9qnI08LsONjEJJPsWPcq2iQNwifyF5c0ewwUpQbTLKKoafn5O8Y/H vggw== X-Gm-Message-State: AOJu0YwJP+bA8POX4Ens454O6m3ED0ffaYpREL2petHBTyYOb8K4n+gy x8laAzKDCzXJbHCEQT+3HFTUjUkbAuiDsw6HftV+WA4zp3N32wqlCZDU X-Gm-Gg: Acq92OHHQpL5gtdnRwipw3rcKkfSwpfX0yzDUBMI0NfZKMiZCmsj035qyEb0Mvaf9w5 DC6VyFKuv1S2y5sTIrdN4TLPjzzo+4lieaKaBeQsoWz/CFAqzm3vlMMyax7tSuuxZyJB85S9smt jqnoFcKN9ZeOuUNlcvf1Ndv/4TPiBVo8jbEpm8ASX1n1Exyflr/L1OBW92agKH/mCnjmlChZqYr EEHxvPZJ+eV03TPMg9NJZ7p5bzGXGa8W1/IEm0WCMc3Urv8V5GQROzPFMzaNryYb4mVpOJyIioN jrHKVDGkVVsxcBI0WjZnj+WiwndmEs/LqMRT4HkaCq3uAkhxk4vm1QcDr1jXV+l3s4sydbW5V2c oF1CZdvn9jtPgitRwbKWA5z55t1YK4GdxNIT40ESBpGpi9mctxUQCpf5Gc3lzKUe3dRlfltr4Kb FSEP2hHrgKIymC+0SW4ODEdB4jOQE= X-Received: by 2002:a05:6000:1a8e:b0:441:202e:3d2d with SMTP id ffacd0b85a97d-45e5c59a3fbmr18364490f8f.19.1779056485427; Sun, 17 May 2026 15:21:25 -0700 (PDT) Received: from grain.localdomain ([212.46.38.55]) by smtp.gmail.com with ESMTPSA id ffacd0b85a97d-45d9ec3acf7sm32072217f8f.12.2026.05.17.15.21.24 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Sun, 17 May 2026 15:21:24 -0700 (PDT) Received: by grain.localdomain (Postfix, from userid 1000) id DD7CD5A005B; Mon, 18 May 2026 01:21:21 +0300 (MSK) Date: Mon, 18 May 2026 01:21:21 +0300 From: Cyrill Gorcunov To: Simon Horman Cc: linux-kernel@vger.kernel.org, netdev@vger.kernel.org, anthony.l.nguyen@intel.com, przemyslaw.kitszel@intel.com Subject: [PATCH v2] ice: Fix wrong dsn read in ice_adapter_put Message-ID: References: <20260517125307.287246-1-horms@kernel.org> Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20260517125307.287246-1-horms@kernel.org> User-Agent: Mutt/2.3.1 (2026-03-20) Content-Transfer-Encoding: quoted-printable Content-Type: text/plain; charset="utf-8" On Sun, May 17, 2026 at 01:53:07PM +0100, Simon Horman wrote: ... > > void ice_adapter_put(struct pci_dev *pdev) > > { > > + const struct ice_pf *pf =3D pci_get_drvdata(pdev); > > + unsigned long index =3D pf->adapter->index; >=20 > Is it possible for pf->adapter to be NULL here if the device was probed in > firmware recovery mode? >=20 > If the device enters firmware recovery mode during ice_probe(), the driver > calls ice_probe_recovery_mode() and skips the ice_adapter_get() allocatio= n. > If the device is subsequently unplugged, the memory-mapped read for the > firmware state register might return 0. >=20 > This would cause ice_is_recovery_mode() to evaluate to false in ice_remov= e(), > allowing the normal teardown sequence to proceed and call ice_adapter_put= (). > Would the unconditional dereference of pf->adapter->index then cause a NU= LL > pointer dereference? If we're in recovery mode the ice_remove will not reach ice_adapter_put, instead it will stop service work and just deinit devlink interface, no? > Also, does this implicit cast to unsigned long bypass the XOR folding used > during insertion on 32-bit architectures? >=20 > The ice_adapters XArray is keyed by an unsigned long index. During insert= ion > in ice_adapter_get(), the index is computed using ice_adapter_xa_index(pd= ev). > On 32-bit architectures, this helper explicitly applies a bitwise XOR fold > to the 64-bit Device Serial Number: >=20 > (u32)index ^ (u32)(index >> 32) >=20 > Since pf->adapter->index is a 64-bit value, assigning it directly to a > 32-bit unsigned long implicitly truncates the upper 32 bits, omitting the > XOR operation. >=20 > Because the lookup index would differ from the insertion index, xa_load() > might fail to find the adapter. Would this trigger the WARN_ON and return > early, permanently leaking the adapter memory? That's the good point but it seems the problem is deeper -- the xor doesn't prevent from index collision which may lead to wrong assumption with shared clocks as far as I can tell (and this will cause warn-on-once in ice_adapte= r_get with unpredictable state of adapter in further work). So it looks that xori= ng index on 32bit archs is just broken. Still point is correct (i've been working with this adapters on 64bit archs only so didn't get this issue). Here is an updated version. --- From: Cyrill Gorcunov Subject: [PATCH v2] ice: Fix wrong dsn read in ice_adapter_put When registering an adapter instance, we read the PCI configuration space to fetch the DSN and generate an adapter index for lookups. However, if the adapter has been physically unplugged, the PCI space is no longer accessible. Reading it returns a zero value, which results in either an incorrect adapter instance being put or the proper instance not being put at all. To fix this, we will use the previously known index instead. Signed-off-by: Cyrill Gorcunov --- drivers/net/ethernet/intel/ice/ice_adapter.c | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) Index: linux-tip.git/drivers/net/ethernet/intel/ice/ice_adapter.c =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D --- linux-tip.git.orig/drivers/net/ethernet/intel/ice/ice_adapter.c +++ linux-tip.git/drivers/net/ethernet/intel/ice/ice_adapter.c @@ -40,10 +40,8 @@ static u64 ice_adapter_index(struct pci_ } } =20 -static unsigned long ice_adapter_xa_index(struct pci_dev *pdev) +static unsigned long xa_index_mangle(u64 index) { - u64 index =3D ice_adapter_index(pdev); - #if BITS_PER_LONG =3D=3D 64 return index; #else @@ -51,6 +49,12 @@ static unsigned long ice_adapter_xa_inde #endif } =20 +static unsigned long ice_adapter_xa_index(struct pci_dev *pdev) +{ + u64 index =3D ice_adapter_index(pdev); + return xa_index_mangle(index); +} + static struct ice_adapter *ice_adapter_new(struct pci_dev *pdev) { struct ice_adapter *adapter; @@ -130,13 +134,13 @@ struct ice_adapter *ice_adapter_get(stru */ void ice_adapter_put(struct pci_dev *pdev) { + const struct ice_pf *pf =3D pci_get_drvdata(pdev); + unsigned long index =3D xa_index_mangle(pf->adapter->index); struct ice_adapter *adapter; - unsigned long index; =20 - index =3D ice_adapter_xa_index(pdev); scoped_guard(mutex, &ice_adapters_mutex) { adapter =3D xa_load(&ice_adapters, index); - if (WARN_ON(!adapter)) + if (WARN_ON(!adapter || adapter !=3D pf->adapter)) return; if (!refcount_dec_and_test(&adapter->refcount)) return;