Re: net: Zeroing the structure ethtool_wolinfo in ethtool_get_wol()

On 23 August 2016 at 17:05, Joe Perches [off-list ref] wrote:

On Tue, 2016-08-23 at 07:21 -0700, Eric Dumazet wrote:

quoted

On Tue, 2016-08-23 at 14:41 +0100, Luis Henriques wrote:

quoted

From: Avijit Kanti Das <redacted>

memset() the structure ethtool_wolinfo that has padded bytes
but the padded bytes have not been zeroed out.

[]

quoted

quoted

diff --git a/net/core/ethtool.c b/net/core/ethtool.c

[]

quoted

quoted

@@ -1435,11 +1435,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)))

This would suggest a compiler bug to me.

A compiler does not have a standards based requirement to
initialize arbitrary padding bytes.

I believe gcc always does zero all padding anyway.

quoted

I checked that my compiler does properly put zeros there, even in the
padding area.

If we can not rely on such constructs, we have hundreds of similar
patches to submit.

True.

From a practical point of view, does any compiler used for
kernel compilation (gcc/icc/llvm/any others?) not always
perform zero padding of alignment bytes?

gcc often does not do it, depends on a few factors though:

https://lkml.org/lkml/2016/5/20/389


Vegard