** Description changed:

- Upstream linux kernel implemented ring parameter boundaries check in commit:
- 37e2d99b59c4 ("ethtool: Ensure new ring parameters are within bounds during 
SRINGPARAM")
+ [Impact]
+ * There's a bad behavior in the ena driver ringparam setting on kernels 4.4 
and 4.15, if an invalid ringparam is provided to ethtool.
  
- (see link
- https://patchwork.ozlabs.org/project/netdev/patch/1515420026-11970-2
- -git-send-email-tar...@mellanox.com/)
+ * Upstream Linux kernel implemented ring parameter boundaries check in 
commit: 37e2d99b59c4 ("ethtool: Ensure new ring parameters are within bounds 
during SRINGPARAM") [ git.kernel.org/linus/37e2d99b59c4 ].
+ Due to this commit, the community doesn't usually allow ring parameter 
boundary checks in driver code.
  
- Due to this commit, the community doesn't allow ring parameter boundary
- checks in driver code.
+ * Xenial/Bionic kernels don't include this patch, and some network
+ drivers (like ena) rely on this patch for boundary checking of ring
+ params. So, we are hereby requesting the commit inclusion in these
+ kernel versions.
  
- The Bionic ubuntu kernel does not include this patch. And some network 
drivers rely on this patch for
- boundary checking of ring params.
+ [Test case]
+ 1. In AWS, create a new c5.4xlarge instance with the Ubuntu 18.04 official 
ami (uses the ENA network driver) and update to latest kernel/reboot.
  
- This causes bugs in case a ringparam was given an illegal value.
- 
- Reproduction steps in AWS:
- 
- 1. Create new c5.4xlarge instance with the ubuntu 18.04 official ami (uses 
the ENA network driver) and update to latest kernel using apt-get dist-upgrade 
and reboot for the new kernel to load.
  2. Run ethtool -g ens5
  output:
  Ring parameters for ens5:
  Pre-set maximums:
  RX:           16384
  RX Mini:      0
  RX Jumbo:     0
  TX:           1024
  Current hardware settings:
  RX:           1024
  RX Mini:      0
  RX Jumbo:     0
  TX:           1024
  
- 3. Change the tx ring size to a legal number within boundaries - works!
- 4. Change the tx ring size to an illegal number such as 2048 with the command 
- sudo ethtool -G ens5 tx 2048.
- Expected behavior - operation not allowed
- Actual behavior - causes a crash of the driver since boundaries are not 
checked by ethtool.
+ 3. Change the TX/RX ring size to a legal number within boundaries -
+ works!
  
- Proposed fix - include the above commit 37e2d99b59c4 ("ethtool: Ensure
- new ring parameters are within bounds during SRINGPARAM") in bionic (and
- probably other ubuntu releases as well)
+ 4. Change the TX/RX ring size to an illegal number (such as 2048 for TX)
+ with the command - "sudo ethtool -G ens5 tx 2048".
  
+ Expected behavior - "Cannot set device ring parameters: Invalid argument"
+ Actual behavior - causes a driver hang since boundaries are not checked by 
ethtool, effectively hanging the instance (given that AWS has no console to 
allow system manipulation).
  
- More data about the machine where the bug occured:
+ [Regression Potential]
  
- ubuntu@ip-172-31-80-28:~/bionic$ lsb_release -rd
- Description:  Ubuntu 18.04.4 LTS
- Release:      18.04
+ Since that the commit is present in kernels v4.16+ (including Ubuntu)
+ and is quite small and self-contained, the regression risk is very
+ reduced.
  
- ubuntu@ip-172-31-80-28:~/bionic$ uname -r
- 4.15.0-1065-aws
+ One potential "regression" would be if some driver has bugs and provide
+ bad values on get_ringparams, then the validation would be broken
+ (allowing illegal values or refusing legal ones), but this wouldn't be a
+ regression in the hereby proposed patch itself, it'd be only exposed by
+ the patch.

-- 
You received this bug notification because you are a member of Kernel
Packages, which is subscribed to linux in Ubuntu.
https://bugs.launchpad.net/bugs/1874444

Title:
  Bionic ubuntu ethtool doesn't check ring parameters boundaries

Status in linux package in Ubuntu:
  In Progress
Status in linux source package in Xenial:
  In Progress
Status in linux source package in Bionic:
  In Progress

Bug description:
  [Impact]
  * There's a bad behavior in the ena driver ringparam setting on kernels 4.4 
and 4.15, if an invalid ringparam is provided to ethtool.

  * Upstream Linux kernel implemented ring parameter boundaries check in 
commit: 37e2d99b59c4 ("ethtool: Ensure new ring parameters are within bounds 
during SRINGPARAM") [ git.kernel.org/linus/37e2d99b59c4 ].
  Due to this commit, the community doesn't usually allow ring parameter 
boundary checks in driver code.

  * Xenial/Bionic kernels don't include this patch, and some network
  drivers (like ena) rely on this patch for boundary checking of ring
  params. So, we are hereby requesting the commit inclusion in these
  kernel versions.

  [Test case]
  1. In AWS, create a new c5.4xlarge instance with the Ubuntu 18.04 official 
ami (uses the ENA network driver) and update to latest kernel/reboot.

  2. Run ethtool -g ens5
  output:
  Ring parameters for ens5:
  Pre-set maximums:
  RX:           16384
  RX Mini:      0
  RX Jumbo:     0
  TX:           1024
  Current hardware settings:
  RX:           1024
  RX Mini:      0
  RX Jumbo:     0
  TX:           1024

  3. Change the TX/RX ring size to a legal number within boundaries -
  works!

  4. Change the TX/RX ring size to an illegal number (such as 2048 for
  TX) with the command - "sudo ethtool -G ens5 tx 2048".

  Expected behavior - "Cannot set device ring parameters: Invalid argument"
  Actual behavior - causes a driver hang since boundaries are not checked by 
ethtool, effectively hanging the instance (given that AWS has no console to 
allow system manipulation).

  [Regression Potential]

  Since that the commit is present in kernels v4.16+ (including Ubuntu)
  and is quite small and self-contained, the regression risk is very
  reduced.

  One potential "regression" would be if some driver has bugs and
  provide bad values on get_ringparams, then the validation would be
  broken (allowing illegal values or refusing legal ones), but this
  wouldn't be a regression in the hereby proposed patch itself, it'd be
  only exposed by the patch.

To manage notifications about this bug go to:
https://bugs.launchpad.net/ubuntu/+source/linux/+bug/1874444/+subscriptions

-- 
Mailing list: https://launchpad.net/~kernel-packages
Post to     : kernel-packages@lists.launchpad.net
Unsubscribe : https://launchpad.net/~kernel-packages
More help   : https://help.launchpad.net/ListHelp

Reply via email to