dl2k: Tighten ioctl permissions
Subsystems (2)
PATCH 1/1 dl2k: Tighten ioctl permissions
dl2k's rio_ioctl function defines several ioctls that involve operations that should be denied to regular users.
SIOCDEVPRIVATE + 2 is a renumbered SIOCSMIIREG.
SIOCDEVPRIVATE + 5 calls netif_stop_queue.
SIOCDEVPRIVATE + 6 calls netif_wake_queue.
--- drivers/net/ethernet/dlink/dl2k.c | 8 ++++++++ 1 file changed, 8 insertions(+)
drivers/net/ethernet/dlink/dl2k.c +8 -0
@@ -1264,6 +1264,14 @@ rio_ioctl (struct net_device *dev, strucstruct netdev_desc *desc;int i; +switch (cmd) { +case SIOCDEVPRIVATE +2 : +case SIOCDEVPRIVATE +5 : +case SIOCDEVPRIVATE +6 : +if (!capable (CAP_NET_ADMIN )) +return -EPERM ; + }; + phy_addr = np->phy_addr;switch (cmd) {case SIOCDEVPRIVATE :
Signature · 2 lines
Jeff Mahoney SUSE Labs
Quoted 4 lines · Jeff Mahoney
dl2k's
rio_ioctlfunction defines several ioctls that involve
operations that should be denied to regular users.SIOCDEVPRIVATE + 2 is a renumbered SIOCSMIIREG.
There was an early convention that SIOCDEVPRIVATE + {0,1,2} were MDIO operations. (This was a bad idea, because you can't safely send them to an arbitrary driver... not that that stopped people doing it. Now it's neither safe to send them from userland, nor to implement any other semantics for these ioctl numbers in a driver.)
Please fix the numbering instead; it will make standard MII/MDIO utilities work and the capability check (in dev_ioctl()) comes for free.
Quoted 2 lines
SIOCDEVPRIVATE + 5 calls
netif_stop_queue.
SIOCDEVPRIVATE + 6 callsnetif_wake_queue.
[...]
And SIOCDEVPRIVATE + {7,8} spam the kernel log, so they should perhaps be considered privileged too.
Ben.
Signature · 3 lines
Ben Hutchings, Staff Engineer, Solarflare Not speaking for my employer; that's the marketing department's job. They asked us to note that Solarflare product names are trademarked.
David Miller
maintainerFrom: Ben Hutchings <[email protected]>
Date: Wed, 25 Apr 2012 23:26:42 +0100
Quoted 15 lines · Jeff Mahoney
Quoted 4 lines
dl2k's
rio_ioctlfunction defines several ioctls that involve
operations that should be denied to regular users.SIOCDEVPRIVATE + 2 is a renumbered SIOCSMIIREG.
There was an early convention that SIOCDEVPRIVATE + {0,1,2} were MDIO operations. (This was a bad idea, because you can't safely send them to an arbitrary driver... not that that stopped people doing it. Now it's neither safe to send them from userland, nor to implement any other semantics for these ioctl numbers in a driver.)
Please fix the numbering instead; it will make standard MII/MDIO utilities work and the capability check (in
dev_ioctl()) comes for free.
Quoted 6 lines
Quoted 2 lines
SIOCDEVPRIVATE + 5 calls
netif_stop_queue.
SIOCDEVPRIVATE + 6 callsnetif_wake_queue.[...]
And SIOCDEVPRIVATE + {7,8} spam the kernel log, so they should perhaps be considered privileged too.
And I would also say that the netif_{stop,wake}_queue ones should just be deleted. There is no sane way, even as a debugging facility, we can let useland trigger these conditions.
And if we could, it belongs in a generic facility no private ioctls which are heavily discouraged anyways.
I seriously would suggest ditching all of these private ioctls from the dl2k driver, I bet no binary in the world out there even exists which handles these strange renumbered MII operations.
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
Quoted 31 lines · David Miller
From: Ben Hutchings <[email protected]> Date: Wed, 25 Apr 2012 23:26:42 +0100
Quoted 17 lines · Jeff Mahoney
Quoted 4 lines
dl2k's
rio_ioctlfunction defines several ioctls that involve operations that should be denied to regular users.SIOCDEVPRIVATE + 2 is a renumbered SIOCSMIIREG.
There was an early convention that SIOCDEVPRIVATE + {0,1,2} were MDIO operations. (This was a bad idea, because you can't safely send them to an arbitrary driver... not that that stopped people doing it. Now it's neither safe to send them from userland, nor to implement any other semantics for these ioctl numbers in a driver.)
Please fix the numbering instead; it will make standard MII/MDIO utilities work and the capability check (in
dev_ioctl()) comes for free.Quoted 6 lines
Quoted 2 lines
SIOCDEVPRIVATE + 5 calls
netif_stop_queue. SIOCDEVPRIVATE + 6 callsnetif_wake_queue.[...]
And SIOCDEVPRIVATE + {7,8} spam the kernel log, so they should perhaps be considered privileged too.
And I would also say that the netif_{stop,wake}_queue ones should just be deleted. There is no sane way, even as a debugging facility, we can let useland trigger these conditions.
Quoted 6 lines
And if we could, it belongs in a generic facility no private ioctls which are heavily discouraged anyways.
I seriously would suggest ditching all of these private ioctls from the dl2k driver, I bet no binary in the world out there even exists which handles these strange renumbered MII operations.
Ok. I expected this is what you'd say, but I don't usually play in the network code or have one of these devices to do any testing with. I'll kill off the private ioctls and switch the MDIO ones to the standard interface.
- -Jeff
- --
Jeff Mahoney
SUSE Labs
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2.0.19 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/
iQIcBAEBAgAGBQJPmHyMAAoJEB57S2MheeWyC2YQAIiUSuC3TTQkFW0sDdrGIJtB BSfxeSJVYN6SOmKWK9G6/AsTciB/9z/d3lQ6duJQCFTZCnUcCWcKN81iIHRJVZ32 5f97MBnPnSbZvjGyaHT3QNodH6Jq0uNQRHAXm5py7RZpDGmYCFuEr/h5QRibbiH2 IoYG3QMSGUZysOsqYqzk1aHokSfp8CTkoUKHAAyvbo911M6eFO/V6vEQs2ujojDT PZfXGYVqkegOvC3HyGjDuQaoMpUYQC1Uz343mNB6ZJVOM7SCVfN2R4cJGCD6K78s dxbVl68jCfdI8cB5xv1vmWCsZroRlgjxmCnwyM/ecz0IAtmeGNjQXLoQF67WXcV+ hgUgVpcFK0bqCgrtkR1wxMdmloQHlXVf53FFxhIjkJc/KuSalhqdbMdMzKbcGtU/ aqx+1rl937sr8Ffg82qzpUqsunTDmQMV1EB87CdW5ma2ZkjekA3dn8za74OlhP3M hYzytw7Z9rQQt0v5a2FAvYopLDMWgtl8bEgJf2Dnhyrnez0E4iDzIdQq6oYbtipQ xndTUTmHJvIGbJfhtfiUZgG3LhKbFR6a+nz+P+Pk7TRqcuFUTaak0SkCjTBUdPGO TIO2GqFJeAQR3s8F2na2AVzXHXeQrNoWaNlhU0EOt9GARSc+WjftApDbSrB+chKm DP+N7odTX93FIzZ2/chy
=8eUd
-----END PGP SIGNATURE-----