netdev - Re: [PATCH iwl-net v2] ice: use DSN instead of PCI BDF for ice

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: <34b177b1-4be4-4888-a1d5-8b2b6f5f66f5@intel.com>
Date: Mon, 7 Apr 2025 15:26:38 +0200
From: Przemek Kitszel <przemyslaw.kitszel@...el.com>
To: Jiri Pirko <jiri@...nulli.us>
CC: <intel-wired-lan@...ts.osuosl.org>, Tony Nguyen
	<anthony.l.nguyen@...el.com>, <netdev@...r.kernel.org>, Jacob Keller
	<jacob.e.keller@...el.com>, Jakub Kicinski <kuba@...nel.org>, "Aleksandr
 Loktionov" <aleksandr.loktionov@...el.com>, Karol Kolacinski
	<karol.kolacinski@...el.com>, Grzegorz Nitka <grzegorz.nitka@...el.com>,
	Michal Schmidt <mschmidt@...hat.com>, Sergey Temerkhanov
	<sergey.temerkhanov@...el.com>
Subject: Re: [PATCH iwl-net v2] ice: use DSN instead of PCI BDF for
 ice_adapter index


>> struct ice_adapter {
>> 	refcount_t refcount;
>> 	/* For access to the GLTSYN_TIME register */
>> 	spinlock_t ptp_gltsyn_time_lock;
>>
>> 	struct ice_pf *ctrl_pf;
>> 	struct ice_port_list ports;
>> +	u64 device_serial_number;
>> };
>>


>> +	index = ice_adapter_index(dsn);
>> 	scoped_guard(mutex, &ice_adapters_mutex) {
>> 		err = xa_insert(&ice_adapters, index, NULL, GFP_KERNEL);
>> 		if (err == -EBUSY) {
>> 			adapter = xa_load(&ice_adapters, index);
>> 			refcount_inc(&adapter->refcount);
>> +			WARN_ON_ONCE(adapter->device_serial_number != dsn);
> 
> Warn and done? How unlikely is this? I mean, can this happen in real
> world? If yes, that's a bug.

Very unlikely, one would have to have weird NVM *and* also running
32bit mode, I don't like complicating the normal flow to be able to
run fine on misconfigured setups. One redundant field in the struct
and an unlikely() branch on the init path already is almost too much
for my liking, but human will get to know immediately.