[Intel-wired-lan] [PATCH] igb: Add MII write support
Jacob Keller
jacob.e.keller at intel.com
Thu Jun 6 16:56:08 UTC 2024
On 6/6/2024 8:41 AM, Andrew Lunn wrote:
> On Wed, Jun 05, 2024 at 01:51:24PM -0700, Jacob Keller wrote:
>>
>>
>> On 6/3/2024 8:10 PM, jackie.jone at alliedtelesis.co.nz wrote:
>>> From: Jackie Jone <jackie.jone at alliedtelesis.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 at alliedtelesis.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?
>
> User space can break the PHY configuration, cause the link to fail,
> all behind the MAC drivers back. The generic version of this call
> tries to see what registers are being written, and update state:
>
> https://elixir.bootlin.com/linux/latest/source/drivers/net/phy/phy.c#L325
>
> But you can still break it.
>
Yea, its extremely easy to break things if you don't know what you're
doing here. So its more a question of "are we ok exposing yet another
way root can brick things?"
>> 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?
>
> Only root is allowed to use it. So it is a classic 'You have the
> option to shoot yourself in the foot'.
>
I don't have an objection to enabling this myself, but I do want to be
cognizant of the way it is viewed in the wider community.
> For the use case being talked about here, there has been a few emails
> one the list about implementing the IEEE 802.3 test modes. But nobody
> has actually got around to doing it. Not that it would help in this
> case, Intel don't use the Linux PHY drivers, which is where i would
> expect to see such code implemented first. Maybe if the "Great Intel
> Ethernet driver refactoring" makes progress, it could swap to using
> the Linux PHY drivers.
>
> Andrew
I remember this coming up several times in the past. I've always tried
to push for it, but so far unsuccessfully.
More information about the Intel-wired-lan
mailing list