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: <20230401121451.4457de9c@kernel.org>
Date:   Sat, 1 Apr 2023 12:14:51 -0700
From:   Jakub Kicinski <kuba@...nel.org>
To:     Vladimir Oltean <vladimir.oltean@....com>
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 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?