ACK/cmt: [PATCH 1/1] net: Zeroing the structure ethtool_wolinfo in ethtool_get_wol()

[Seth Forshee]

On Thu, Jun 08, 2017 at 06:41:15AM -0700, Brad Figg wrote:
> From: Avijit Kanti Das <avijitnsec at codeaurora.org>
> 
> CVE-2014-9900
> 
> memset() the structure ethtool_wolinfo that has padded bytes
> but the padded bytes have not been zeroed out.
> 
> Change-Id: If3fd2d872a1b1ab9521d937b86a29fc468a8bbfe
> Signed-off-by: Avijit Kanti Das <avijitnsec at codeaurora.org>
> Signed-off-by: Brad Figg <brad.figg at canonical.com>
> ---
>  net/core/ethtool.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/net/core/ethtool.c b/net/core/ethtool.c
> index 23b3394..8fc6595 100644
> --- a/net/core/ethtool.c
> +++ b/net/core/ethtool.c
> @@ -1160,11 +1160,13 @@ static int ethtool_reset(struct net_device *dev, char __user *useraddr)
>  
>  static int ethtool_get_wol(struct net_device *dev, char __user *useraddr)
>  {
> -	struct ethtool_wolinfo wol = { .cmd = ETHTOOL_GWOL };
> +	struct ethtool_wolinfo wol;
>  
>  	if (!dev->ethtool_ops->get_wol)
>  		return -EOPNOTSUPP;
>  
> +	memset(&wol, 0, sizeof(struct ethtool_wolinfo));
> +	wol.cmd = ETHTOOL_GWOL;
>  	dev->ethtool_ops->get_wol(dev, &wol);
>  
>  	if (copy_to_user(useraddr, &wol, sizeof(wol)))

I'm not totally convinced there is any bug here. gcc seems to promise
that omitted fields of a designated initialized will be set to 0 [1].
Nothing is said there about padding bytes, not that it looks like there
should be any in that structure.

At any rate, I suppose the patch does no harm and leaves us with no
possible ambiguity about the data which is copied to userspace, so:

Acked-by: Seth Forshee <seth.forshee at canonical.com>

However, given that it's not an upstream patch shouldn't it be SAUCE?

[1] https://gcc.gnu.org/onlinedocs/gcc/Designated-Inits.html