netdev - Re: [PATCH net-next RFC] Add NDOs for hardware timestamp get/set

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: <20230401160829.7tbxnm5l3ke5ggvr@skbuf>
Date:   Sat, 1 Apr 2023 19:08:29 +0300
From:   Vladimir Oltean <vladimir.oltean@....com>
To:     Jakub Kicinski <kuba@...nel.org>
Cc:     Maxim Georgiev <glipus@...il.com>, kory.maincent@...tlin.com,
        netdev@...r.kernel.org, maxime.chevallier@...tlin.com
Subject: Re: [PATCH net-next RFC] Add NDOs for hardware timestamp get/set

On Thu, Mar 30, 2023 at 10:35:19PM -0700, Jakub Kicinski wrote:
> >  	case SIOCSHWTSTAMP:
> >  		err = net_hwtstamp_validate(ifr);
> >  		if (err)
> >  			return err;
> > -		fallthrough;
> > +		return dev_hwtstamp(dev, ifr, cmd);
> 
> Let's refactor this differently, we need net_hwtstamp_validate()
> to run on the same in-kernel copy as we'll send down to the driver.
> If we copy_from_user() twice we may validate a different thing
> than the driver will end up seeing (ToCToU).

I'm not sure I understand this. Since net_hwtstamp_validate() already
contains a copy_from_user() call, don't we already call copy_to_user()
twice (the second time being in all SIOCSHWTSTAMP handlers from drivers)?

Perhaps I don't understand what is it that can change the contents
of the ifreq structure, which would make this a potential issue for
ndo_hwtstamp_set() that isn't an issue for ndo_eth_ioctl()...