linux-kernel - Re: [PATCH] ethtool: fix a kernel infoleak in ethtool_get

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <574EF7DD.9060905@solarflare.com>
Date:	Wed, 1 Jun 2016 15:57:33 +0100
From:	Edward Cree <ecree@...arflare.com>
To:	Kangjie Lu <kangjielu@...il.com>, <davem@...emloft.net>
CC:	<ben@...adent.org.uk>, <kan.liang@...el.com>,
	<netdev@...r.kernel.org>, <linux-kernel@...r.kernel.org>,
	<taesoo@...ech.edu>, <csong84@...ech.edu>,
	Kangjie Lu <kjlu@...ech.edu>
Subject: Re: [PATCH] ethtool: fix a kernel infoleak in ethtool_get_pauseparam

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 <kjlu@...ech.edu>
> ---
>  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;