From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755016AbcFAPF5 (ORCPT ); Wed, 1 Jun 2016 11:05:57 -0400 Received: from nbfkord-smmo01.seg.att.com ([209.65.160.76]:17426 "EHLO nbfkord-smmo01.seg.att.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753952AbcFAPFx (ORCPT ); Wed, 1 Jun 2016 11:05:53 -0400 X-Greylist: delayed 445 seconds by postgrey-1.27 at vger.kernel.org; Wed, 01 Jun 2016 11:05:53 EDT X-MXL-Hash: 574ef9d11f4380cb-dd07a5af6be1052b0429a4d239751dd4dc3a7c9f X-MXL-Hash: 574ef7ee69afb715-bd5ae83db55819c68d6a3a80f3dc36ef77957432 Subject: Re: [PATCH] ethtool: fix a kernel infoleak in ethtool_get_pauseparam To: Kangjie Lu , References: <1464791961-8169-1-git-send-email-kjlu@gatech.edu> CC: , , , , , , Kangjie Lu From: Edward Cree Message-ID: <574EF7DD.9060905@solarflare.com> Date: Wed, 1 Jun 2016 15:57:33 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.1.0 MIME-Version: 1.0 In-Reply-To: <1464791961-8169-1-git-send-email-kjlu@gatech.edu> Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 8bit X-Originating-IP: [10.17.20.45] X-ClientProxiedBy: ocex03.SolarFlarecom.com (10.20.40.36) To ukex01.SolarFlarecom.com (10.17.10.4) X-TM-AS-Product-Ver: SMEX-11.0.0.1191-8.000.1202-22360.000 X-TM-AS-Result: No--4.074400-8.000000-31 X-TM-AS-User-Approved-Sender: No X-TM-AS-User-Blocked-Sender: No X-AnalysisOut: [v=2.1 cv=eds7CuwH c=1 sm=1 tr=0 a=8P+NB+fYZDP74ap4g4d9Kw==] X-AnalysisOut: [:17 a=fVG4DLb5TBsA:10 a=IkcTkHD0fZMA:10 a=pD_ry4oyNxEA:10 ] X-AnalysisOut: [a=kWg9BAtvLvkknD2ZYUgA:9 a=QEXdDO2ut3YA:10] X-Spam: [F=0.5052279183; CM=0.500; S=0.505(2015072901)] X-MAIL-FROM: X-SOURCE-IP: [193.34.186.16] Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 01/06/16 15:39, Kangjie Lu wrote: > The field autoneg of pauseparam is not initialized in some > implementations of get_pauseparam(), but the whole object is > copied to userland. > > Signed-off-by: Kangjie Lu > --- > net/core/ethtool.c | 5 ++++- > 1 file changed, 4 insertions(+), 1 deletion(-) > > diff --git a/net/core/ethtool.c b/net/core/ethtool.c > index f426c5a..84544bd 100644 > --- a/net/core/ethtool.c > +++ b/net/core/ethtool.c > @@ -1723,7 +1723,10 @@ static noinline_for_stack int ethtool_set_channels(struct net_device *dev, > > static int ethtool_get_pauseparam(struct net_device *dev, void __user *useraddr) > { > - struct ethtool_pauseparam pauseparam = { ETHTOOL_GPAUSEPARAM }; AIUI an incomplete compound initialiser will fill all unspecified fields with zeroes of the appropriate type. So this patch is unnecessary. Per C99, ยง6.7.8.21: > If there are fewer initializers in a brace-enclosed list than there are elements or members of an aggregate [...] the remainder of the aggregate shall be initialized implicitly the same as objects that have static storage duration. -Ed > + struct ethtool_pauseparam pauseparam; > + > + memset(&pauseparam, 0, sizeof(pauseparam)); > + pauseparam.cmd = ETHTOOL_GPAUSEPARAM; > > if (!dev->ethtool_ops->get_pauseparam) > return -EOPNOTSUPP;