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

From: Jakub Kicinski <[email-masked]>
Date: Sat, 1 Apr 2023 12:14:51 -0700

On Sat, 1 Apr 2023 21:20:58 +0300 Vladimir Oltean wrote:
> > After this patch we'll be passing an in-kernel-space struct to drivers
> > rather than the ifr they have to copy themselves. I'm saying that we
> > should validate that exact copy, rather than copy, validate, copy, pass
> > to drivers, cause user space may change the values between the two
> > copies.
> > 
> > Unlikely to cause serious bugs but seems like a good code hygiene.
> > 
> > This is only for the drivers converted to the NDO, obviously, 
> > the legacy drivers will still have to copy themselves.  
> 
> Could you answer my second paragraph too, please?
> 
> | 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()...
> 
> I don't disagree with minimizing the number of copy_to_user() calls, but
> I don't understand the ToCToU argument that you're bringing....

As I said, just code hygiene, I haven't looked at the drivers.
Seems too obvious to invest time trying to come up with an exact
scenario.

Do you see a reason not to code this the right way?