netdev - Re: [PATCH iwl-next 4/5] ixgbe: convert to ndo_hwtstamp_get() and ndo_hwtstamp

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: <e7c5748d-5956-47f5-8135-f1d709a7c66a@intel.com>
Date: Mon, 30 Jun 2025 14:05:03 -0700
From: Jacob Keller <jacob.e.keller@...el.com>
To: Vladimir Oltean <vladimir.oltean@....com>
CC: <intel-wired-lan@...ts.osuosl.org>, Tony Nguyen
	<anthony.l.nguyen@...el.com>, <netdev@...r.kernel.org>, Przemek Kitszel
	<przemyslaw.kitszel@...el.com>, Vinicius Costa Gomes
	<vinicius.gomes@...el.com>, Vadim Fedorenko <vadim.fedorenko@...ux.dev>,
	Richard Cochran <richardcochran@...il.com>
Subject: Re: [PATCH iwl-next 4/5] ixgbe: convert to ndo_hwtstamp_get() and
 ndo_hwtstamp_set()

On 6/30/2025 1:56 PM, Vladimir Oltean wrote:
> 
> Ugh :-/ sorry for the trouble, and thanks for doing the hard work of
> characterizing this.
> 

No worries.

> Indeed, my first conversion of ixgbe was in August 2023, as this commit
> can attest:
> https://github.com/vladimiroltean/linux/commit/0351ebf1eee3381ccfba9d31a924d1dd887a316f
> 
> At that time, Przemyslaw's commit fd5ef5203ce6 ("ixgbe: wrap
> netdev_priv() usage") didn't exist, and "struct ixgbe_adapter *adapter =
> netdev_priv(netdev);" was the de facto idiom in the driver, which I then
> replicated two more times, in the new ixgbe_ptp_hwtstamp_set() and
> ixgbe_ptp_hwtstamp_get() functions. Not only did I not notice that this
> change took place, but it also compiled just fine, making me completely
> unsuspecting...
> 

It would be nice if we had a mechanism to provide type safety for the
netdev_priv.. but with C I don't really know of a good way. If you
provide your own macro you can do type checks on that.. but I'm not sure
how else we could implement this to avoid the type issues.

The use of such a wrapper private structure is essentially required
either in devlink code or netdev code because both of the parent structs
want flexible array private members, so they can't both be ixgbe_adapter.

Of course validating with KASAN would have caught this faster.. I am
actually quite surprised that we manage to get into
ice_ptp_set_timestamp_mode() and invoke the register access functions
without immediately crashing.

At least its caught before this merged and caused memory corruption on
production systems!

> Tony, let me know how you would like to proceed.

Download attachment "OpenPGP_signature.asc" of type "application/pgp-signature" (237 bytes)