Re: [Linux Kernel Bug] KASAN: null-ptr-deref Read in generic_hwtstamp_ioctl_lower

On Wed, 29 Oct 2025 18:19:34 +0200
Vladimir Oltean [off-list ref] wrote:

On Wed, Oct 29, 2025 at 11:06:51AM +0100, Kory Maincent wrote:

quoted

Hello Jiaming,

+Vlad

On Wed, 29 Oct 2025 16:45:37 +0800
Jiaming Zhang [off-list ref] wrote:
  

quoted

Dear Linux kernel developers and maintainers,

We are writing to report a null pointer dereference bug discovered in
the net subsystem. This bug is reproducible on the latest version
(v6.18-rc3, commit dcb6fa37fd7bc9c3d2b066329b0d27dedf8becaa).

The root cause is in tsconfig_prepare_data(), where a local
kernel_hwtstamp_config struct (cfg) is initialized using {}, setting
all its members to zero. Consequently, cfg.ifr becomes NULL.

cfg is then passed as: tsconfig_prepare_data() ->
dev_get_hwtstamp_phylib() -> vlan_hwtstamp_get() (via
dev->netdev_ops->ndo_hwtstamp_get) -> generic_hwtstamp_get_lower() ->
generic_hwtstamp_ioctl_lower().

The function generic_hwtstamp_ioctl_lower() assumes cfg->ifr is a
valid pointer and attempts to access cfg->ifr->ifr_ifru. This access
dereferences the NULL pointer, triggering the bug.  

Thanks for spotting this issue!

In the ideal world we would have all Ethernet driver supporting the
hwtstamp_get/set NDOs but that not currently the case.	
Vladimir Oltean was working on this but it is not done yet. 
$ git grep SIOCGHWTSTAMP drivers/net/ethernet | wc -l
16  

Vadim also took the initiative and submitted (is still submitting?) some
more conversions, whereas I lost all steam.

Ok no worry I was simply pointing this out, people will convert it when they
want to use the new netlink API.
 

quoted

quoted

As a potential fix, we can declare a local struct ifreq variable in
tsconfig_prepare_data(), zero-initializing it, and then assigning its
address to cfg.ifr before calling dev_get_hwtstamp_phylib(). This
ensures that functions down the call chain receive a valid pointer.  

If we do that we will have legacy IOCTL path inside the Netlink path and
that's not something we want.
In fact it is possible because the drivers calling
generic_hwtstamp_get/set_lower functions are already converted to hwtstamp
NDOs therefore the NDO check in tsconfig_prepare_data is not working on
these case.  

I remember we had this discussion before.

| This is why I mentioned by ndo_hwtstamp_set() conversion, because
| suddenly it is a prerequisite for any further progress to be done.
| You can't convert SIOCSHWTSTAMP to netlink if there are some driver
| implementations which still use ndo_eth_ioctl(). They need to be
| UAPI-agnostic.

https://lore.kernel.org/netdev/20231122140850.li2mvf6tpo3f2fhh@skbuf/ (local)

I'm not sure what was your agreement with the netdev maintainer
accepting the tsconfig netlink work with unconverted device drivers left
in the tree.

I did like 21th versions and there was not many people active in the reviews.
No one stand against this work.

quoted

IMO the solution is to add a check on the ifr value in the
generic_hwtstamp_set/get_lower functions like that:

int generic_hwtstamp_set_lower(struct net_device *dev,
			       struct kernel_hwtstamp_config *kernel_cfg,
			       struct netlink_ext_ack *extack)
{
...

	/* Netlink path with unconverted lower driver */
	if (!kernel_cfg->ifr)
		return -EOPNOTSUPP;

	/* Legacy path: unconverted lower driver */
	return generic_hwtstamp_ioctl_lower(dev, SIOCSHWTSTAMP, kernel_cfg);
}  

This plugs one hole (two including _get). How many more are there? If
this is an oversight, the entire tree needs to be reviewed for
ndo_hwtstamp_get() / ndo_hwtstamp_test() pointer tests which were used
as an indication that this net device is netlink ready. Stacked
virtual interfaces are netlink-ready only when the entire chain down to
the physical interface is netlink-ready.

I don't see this as a hole. The legacy ioctl path still works.
If people want to use the new Netlink path on their board, yes they need to
convert all the parts of the chain to hwtstamp NDOs. If they don't they will
get now a EOPNOTSUPP error instead of a null pointer dereference koops.

Regards,
-- 
Köry Maincent, Bootlin
Embedded Linux and kernel engineering
https://bootlin.com