| Message ID | 20210325011200.145818-6-kuba@kernel.org (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | ethtool: clarify the ethtool FEC interface | expand |
| Context | Check | Description |
|---|---|---|
| netdev/cover_letter | success | Link |
| netdev/fixes_present | success | Link |
| netdev/patch_count | success | Link |
| netdev/tree_selection | success | Clearly marked for net-next |
| netdev/subject_prefix | success | Link |
| netdev/cc_maintainers | warning | 5 maintainers not CCed: ecree@solarflare.com magnus.karlsson@intel.com f.fainelli@gmail.com danieller@nvidia.com irusskikh@marvell.com |
| netdev/source_inline | success | Was 0 now: 0 |
| netdev/verify_signedoff | success | Link |
| netdev/module_param | success | Was 0 now: 0 |
| netdev/build_32bit | success | Errors and warnings before: 1 this patch: 1 |
| netdev/kdoc | success | Errors and warnings before: 0 this patch: 0 |
| netdev/verify_fixes | success | Link |
| netdev/checkpatch | success | total: 0 errors, 0 warnings, 0 checks, 9 lines checked |
| netdev/build_allmodconfig_warn | success | Errors and warnings before: 1 this patch: 1 |
| netdev/header_inline | success | Link |
Hi Jakub,
url: https://github.com/0day-ci/linux/commits/Jakub-Kicinski/ethtool-clarify-the-ethtool-FEC-interface/20210325-091411
base: https://git.kernel.org/pub/scm/linux/kernel/git/davem/net-next.git 69cdfb530f7b8b094e49555454869afc8140b1bb
config: x86_64-randconfig-m001-20210325 (attached as .config)
compiler: gcc-9 (Debian 9.3.0-22) 9.3.0
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
smatch warnings:
net/ethtool/ioctl.c:2589 ethtool_set_fecparam() warn: bitwise AND condition is false here
vim +2589 net/ethtool/ioctl.c
1a5f3da20bd966 net/core/ethtool.c Vidya Sagar Ravipati 2017-07-27 2579 static int ethtool_set_fecparam(struct net_device *dev, void __user *useraddr)
1a5f3da20bd966 net/core/ethtool.c Vidya Sagar Ravipati 2017-07-27 2580 {
1a5f3da20bd966 net/core/ethtool.c Vidya Sagar Ravipati 2017-07-27 2581 struct ethtool_fecparam fecparam;
1a5f3da20bd966 net/core/ethtool.c Vidya Sagar Ravipati 2017-07-27 2582
1a5f3da20bd966 net/core/ethtool.c Vidya Sagar Ravipati 2017-07-27 2583 if (!dev->ethtool_ops->set_fecparam)
1a5f3da20bd966 net/core/ethtool.c Vidya Sagar Ravipati 2017-07-27 2584 return -EOPNOTSUPP;
1a5f3da20bd966 net/core/ethtool.c Vidya Sagar Ravipati 2017-07-27 2585
1a5f3da20bd966 net/core/ethtool.c Vidya Sagar Ravipati 2017-07-27 2586 if (copy_from_user(&fecparam, useraddr, sizeof(fecparam)))
1a5f3da20bd966 net/core/ethtool.c Vidya Sagar Ravipati 2017-07-27 2587 return -EFAULT;
1a5f3da20bd966 net/core/ethtool.c Vidya Sagar Ravipati 2017-07-27 2588
15beed7dba77ce net/ethtool/ioctl.c Jakub Kicinski 2021-03-24 @2589 if (!fecparam.fec || fecparam.fec & ETHTOOL_FEC_NONE_BIT)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
This should be if (!fecparam.fec || fecparam.fec & BIT(ETHTOOL_FEC_NONE_BIT))
15beed7dba77ce net/ethtool/ioctl.c Jakub Kicinski 2021-03-24 2590 return -EINVAL;
15beed7dba77ce net/ethtool/ioctl.c Jakub Kicinski 2021-03-24 2591
c405852e12f210 net/ethtool/ioctl.c Jakub Kicinski 2021-03-24 2592 fecparam.active_fec = 0;
76d37e2ba4f23d net/ethtool/ioctl.c Jakub Kicinski 2021-03-24 2593 fecparam.reserved = 0;
76d37e2ba4f23d net/ethtool/ioctl.c Jakub Kicinski 2021-03-24 2594
1a5f3da20bd966 net/core/ethtool.c Vidya Sagar Ravipati 2017-07-27 2595 return dev->ethtool_ops->set_fecparam(dev, &fecparam);
1a5f3da20bd966 net/core/ethtool.c Vidya Sagar Ravipati 2017-07-27 2596 }
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
On Thu, 25 Mar 2021 15:00:47 +0300 Dan Carpenter wrote: > Hi Jakub, > > url: https://github.com/0day-ci/linux/commits/Jakub-Kicinski/ethtool-clarify-the-ethtool-FEC-interface/20210325-091411 > base: https://git.kernel.org/pub/scm/linux/kernel/git/davem/net-next.git 69cdfb530f7b8b094e49555454869afc8140b1bb > config: x86_64-randconfig-m001-20210325 (attached as .config) > compiler: gcc-9 (Debian 9.3.0-22) 9.3.0 > > If you fix the issue, kindly add following tag as appropriate > Reported-by: kernel test robot <lkp@intel.com> > Reported-by: Dan Carpenter <dan.carpenter@oracle.com> > > smatch warnings: > net/ethtool/ioctl.c:2589 ethtool_set_fecparam() warn: bitwise AND condition is false here > > vim +2589 net/ethtool/ioctl.c > > 1a5f3da20bd966 net/core/ethtool.c Vidya Sagar Ravipati 2017-07-27 2579 static int ethtool_set_fecparam(struct net_device *dev, void __user *useraddr) > 1a5f3da20bd966 net/core/ethtool.c Vidya Sagar Ravipati 2017-07-27 2580 { > 1a5f3da20bd966 net/core/ethtool.c Vidya Sagar Ravipati 2017-07-27 2581 struct ethtool_fecparam fecparam; > 1a5f3da20bd966 net/core/ethtool.c Vidya Sagar Ravipati 2017-07-27 2582 > 1a5f3da20bd966 net/core/ethtool.c Vidya Sagar Ravipati 2017-07-27 2583 if (!dev->ethtool_ops->set_fecparam) > 1a5f3da20bd966 net/core/ethtool.c Vidya Sagar Ravipati 2017-07-27 2584 return -EOPNOTSUPP; > 1a5f3da20bd966 net/core/ethtool.c Vidya Sagar Ravipati 2017-07-27 2585 > 1a5f3da20bd966 net/core/ethtool.c Vidya Sagar Ravipati 2017-07-27 2586 if (copy_from_user(&fecparam, useraddr, sizeof(fecparam))) > 1a5f3da20bd966 net/core/ethtool.c Vidya Sagar Ravipati 2017-07-27 2587 return -EFAULT; > 1a5f3da20bd966 net/core/ethtool.c Vidya Sagar Ravipati 2017-07-27 2588 > 15beed7dba77ce net/ethtool/ioctl.c Jakub Kicinski 2021-03-24 @2589 if (!fecparam.fec || fecparam.fec & ETHTOOL_FEC_NONE_BIT) > ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ :o good catch. s/_BIT//. Thanks!
diff --git a/net/ethtool/ioctl.c b/net/ethtool/ioctl.c index 237ffe5440ef..8797533ddc4b 100644 --- a/net/ethtool/ioctl.c +++ b/net/ethtool/ioctl.c @@ -2582,14 +2582,17 @@ static int ethtool_set_fecparam(struct net_device *dev, void __user *useraddr) if (!dev->ethtool_ops->set_fecparam) return -EOPNOTSUPP; if (copy_from_user(&fecparam, useraddr, sizeof(fecparam))) return -EFAULT; + if (!fecparam.fec || fecparam.fec & ETHTOOL_FEC_NONE_BIT) + return -EINVAL; + fecparam.active_fec = 0; fecparam.reserved = 0; return dev->ethtool_ops->set_fecparam(dev, &fecparam); } /* The main entry point in this file. Called from net/core/dev_ioctl.c */
Reject NONE on set, this mode means device does not support FEC so it's a little out of place in the set interface. This should be safe to do - user space ethtool does not allow the use of NONE on set. A few drivers treat it the same as OFF, but none use it instead of OFF. Similarly reject an empty FEC mask. The common user space tool will not send such requests and most drivers correctly reject it already. Signed-off-by: Jakub Kicinski <kuba@kernel.org> --- net/ethtool/ioctl.c | 3 +++ 1 file changed, 3 insertions(+)