From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jakub Kicinski Subject: Re: [PATCH net-next 1/3] ethtool: Ensure new ring parameters are within bounds during SRINGPARAM Date: Mon, 8 Jan 2018 18:23:42 -0800 Message-ID: <20180108182342.084c5d60@cakuba.netronome.com> References: <1515420026-11970-1-git-send-email-tariqt@mellanox.com> <1515420026-11970-2-git-send-email-tariqt@mellanox.com> Mime-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Cc: "David S. Miller" , netdev@vger.kernel.org, Eran Ben Elisha , Eugenia Emantayev To: Tariq Toukan Return-path: Received: from mx3.wp.pl ([212.77.101.9]:42672 "EHLO mx3.wp.pl" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750742AbeAICXs (ORCPT ); Mon, 8 Jan 2018 21:23:48 -0500 In-Reply-To: <1515420026-11970-2-git-send-email-tariqt@mellanox.com> Sender: netdev-owner@vger.kernel.org List-ID: On Mon, 8 Jan 2018 16:00:24 +0200, Tariq Toukan wrote: > From: Eugenia Emantayev > > Add a sanity check to ensure that all requested ring parameters > are within bounds, which should reduce errors in driver implementation. (y) > Signed-off-by: Eugenia Emantayev > Signed-off-by: Tariq Toukan > --- > net/core/ethtool.c | 13 +++++++++++-- > 1 file changed, 11 insertions(+), 2 deletions(-) > > diff --git a/net/core/ethtool.c b/net/core/ethtool.c > index 50a79203043b..9ea7cd52fde0 100644 > --- a/net/core/ethtool.c > +++ b/net/core/ethtool.c > @@ -1704,14 +1704,23 @@ static int ethtool_get_ringparam(struct net_device *dev, void __user *useraddr) > > static int ethtool_set_ringparam(struct net_device *dev, void __user *useraddr) > { > - struct ethtool_ringparam ringparam; > + struct ethtool_ringparam ringparam, max = { .cmd = ETHTOOL_GRINGPARAM }; > > - if (!dev->ethtool_ops->set_ringparam) > + if (!dev->ethtool_ops->set_ringparam || !dev->ethtool_ops->get_ringparam) > return -EOPNOTSUPP; > > if (copy_from_user(&ringparam, useraddr, sizeof(ringparam))) > return -EFAULT; > > + dev->ethtool_ops->get_ringparam(dev, &max); Perhaps check the return value here? It's pretty unlikely but get_ringparam may fail. > + /* ensure new ring parameters are within the maximums */ > + if (ringparam.rx_pending > max.rx_max_pending || > + ringparam.rx_mini_pending > max.rx_mini_max_pending || > + ringparam.rx_jumbo_pending > max.rx_jumbo_max_pending || > + ringparam.tx_pending > max.tx_max_pending) > + return -EINVAL; > + > return dev->ethtool_ops->set_ringparam(dev, &ringparam); > } >