[Intel-wired-lan] [PATCH] igb: Add MII write support

Loktionov, Aleksandr aleksandr.loktionov at intel.com
Thu Jun 6 04:30:02 UTC 2024



> -----Original Message-----
> From: Intel-wired-lan <intel-wired-lan-bounces at osuosl.org> On
> Behalf Of Jacob Keller
> Sent: Wednesday, June 5, 2024 11:17 PM
> To: Chris Packham <Chris.Packham at alliedtelesis.co.nz>; Jackie Jone
> <Jackie.Jone at alliedtelesis.co.nz>; davem at davemloft.net
> Cc: netdev at vger.kernel.org; linux-kernel at vger.kernel.org; Nguyen,
> Anthony L <anthony.l.nguyen at intel.com>; intel-wired-
> lan at lists.osuosl.org; kuba at kernel.org
> Subject: Re: [Intel-wired-lan] [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 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?
> >>
> >> 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 pretty sure from experience that even root applications will start cause nvmupdate issues.

> 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?


More information about the Intel-wired-lan mailing list