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

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: <20251029161934.xwxzqoknqmwtrsgv@skbuf>
Date: Wed, 29 Oct 2025 18:19:34 +0200
From: Vladimir Oltean <vladimir.oltean@....com>
To: Kory Maincent <kory.maincent@...tlin.com>
Cc: Jiaming Zhang <r772577952@...il.com>, davem@...emloft.net,
	edumazet@...gle.com, kuba@...nel.org, netdev@...r.kernel.org,
	pabeni@...hat.com, horms@...nel.org, kuniyu@...gle.com,
	linux-kernel@...r.kernel.org, sdf@...ichev.me,
	syzkaller@...glegroups.com,
	Vadim Fedorenko <vadim.fedorenko@...ux.dev>
Subject: Re: [Linux Kernel Bug] KASAN: null-ptr-deref Read in
 generic_hwtstamp_ioctl_lower

On Wed, Oct 29, 2025 at 11:06:51AM +0100, Kory Maincent wrote:
> Hello Jiaming,
> 
> +Vlad
> 
> On Wed, 29 Oct 2025 16:45:37 +0800
> Jiaming Zhang <r772577952@...il.com> wrote:
> 
> > 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.

> > 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/

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.

> 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.