netdev - Re: [PATCH] igb: Add MII write support

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]

Date: Wed, 5 Jun 2024 14:16:47 -0700
From: Jacob Keller <jacob.e.keller@...el.com>
To: Chris Packham <Chris.Packham@...iedtelesis.co.nz>, Jackie Jone
	<Jackie.Jone@...iedtelesis.co.nz>, "davem@...emloft.net"
	<davem@...emloft.net>
CC: "jesse.brandeburg@...el.com" <jesse.brandeburg@...el.com>,
	"anthony.l.nguyen@...el.com" <anthony.l.nguyen@...el.com>, "kuba@...nel.org"
	<kuba@...nel.org>, "intel-wired-lan@...ts.osuosl.org"
	<intel-wired-lan@...ts.osuosl.org>, "netdev@...r.kernel.org"
	<netdev@...r.kernel.org>, "linux-kernel@...r.kernel.org"
	<linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] igb: Add MII write support



On 6/5/2024 2:10 PM, Chris Packham wrote:
> 
> On 6/06/24 08:51, Jacob Keller wrote:
>>
>> On 6/3/2024 8:10 PM, jackie.jone@...iedtelesis.co.nz wrote:
>>> From: Jackie Jone <jackie.jone@...iedtelesis.co.nz>
>>>
>>> To facilitate running PHY parametric tests, add support for the SIOCSMIIREG
>>> ioctl. This allows a userspace application to write to the PHY registers
>>> to enable the test modes.
>>>
>>> Signed-off-by: Jackie Jone <jackie.jone@...iedtelesis.co.nz>
>>> ---
>>>   drivers/net/ethernet/intel/igb/igb_main.c | 4 ++++
>>>   1 file changed, 4 insertions(+)
>>>
>>> diff --git a/drivers/net/ethernet/intel/igb/igb_main.c b/drivers/net/ethernet/intel/igb/igb_main.c
>>> index 03a4da6a1447..7fbfcf01fbf9 100644
>>> --- a/drivers/net/ethernet/intel/igb/igb_main.c
>>> +++ b/drivers/net/ethernet/intel/igb/igb_main.c
>>> @@ -8977,6 +8977,10 @@ static int igb_mii_ioctl(struct net_device *netdev, struct ifreq *ifr, int cmd)
>>>   			return -EIO;
>>>   		break;
>>>   	case SIOCSMIIREG:
>>> +		if (igb_write_phy_reg(&adapter->hw, data->reg_num & 0x1F,
>>> +				     data->val_in))
>>> +			return -EIO;
>>> +		break;
>> A handful of drivers seem to expose this. What are the consequences of
>> exposing this ioctl? What can user space do with it?
>>
>> It looks like a few drivers also check something like CAP_NET_ADMIN to
>> avoid allowing write access to all users. Is that enforced somewhere else?
> 
> CAP_NET_ADMIN is enforced via dev_ioctl() so it should already be 
> restricted to users with that capability.

Ok good. That at least limits this so that random users can't cause any
side effects.

I'm not super familiar with what can be affected by writing the MII
registers. I'm also not sure what the community thinks of exposing such
access directly.

>From the description this is intended to use for debugging and testing
purposes?