> -----Original Message----- > From: Intel-wired-lan <intel-wired-lan-boun...@osuosl.org> On > Behalf Of Jacob Keller > Sent: Wednesday, June 5, 2024 11:17 PM > To: Chris Packham <chris.pack...@alliedtelesis.co.nz>; Jackie Jone > <jackie.j...@alliedtelesis.co.nz>; da...@davemloft.net > Cc: net...@vger.kernel.org; linux-ker...@vger.kernel.org; Nguyen, > Anthony L <anthony.l.ngu...@intel.com>; intel-wired- > l...@lists.osuosl.org; k...@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.j...@alliedtelesis.co.nz wrote: > >>> From: Jackie Jone <jackie.j...@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.j...@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?
