ethtool: zero initialize coalesce struct

Message ID 20181215011923.246179-1-zenczykowski@gmail.com
State Accepted
Delegated to: John Linville
Headers show
Series
  • ethtool: zero initialize coalesce struct
Related show

Commit Message

Maciej Żenczykowski Dec. 15, 2018, 1:19 a.m.
From: Maciej Żenczykowski <maze@google.com>

prior to fetching it from kernel.

Otherwise we run the risk of very tail portion of it (dmac field)
being left entirely uninitialized, and likely containing some sort
of stale data.

It seems to likely be some sort of time (a second's counter).

Tested:
  'ethtool -c eth1' with old kernel now reports 'dmac: 0' where
  previously it reported some sort of second counter.

Signed-off-by: Maciej Żenczykowski <maze@google.com>
---
 ethtool.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Michal Kubecek Dec. 15, 2018, 2:26 a.m. | #1
On Fri, Dec 14, 2018 at 05:19:23PM -0800, Maciej Żenczykowski wrote:
> From: Maciej Żenczykowski <maze@google.com>
> 
> prior to fetching it from kernel.
> 
> Otherwise we run the risk of very tail portion of it (dmac field)
> being left entirely uninitialized, and likely containing some sort
> of stale data.

The dmac member of struct ethtool_coalesce was removed by commit
c0b078ce7e88 ("Revert "ethtool: Add DMA Coalescing support"") because it
has never been added in kernel (as that would break compatibility with
older ethtool versions).

Michal Kubecek
Maciej Żenczykowski Dec. 15, 2018, 2:30 a.m. | #2
> The dmac member of struct ethtool_coalesce was removed by commit
> c0b078ce7e88 ("Revert "ethtool: Add DMA Coalescing support"") because it
> has never been added in kernel (as that would break compatibility with
> older ethtool versions).

Hmm, well, it doesn't hurt to zero init either way...
Ben Hutchings Dec. 15, 2018, 12:29 p.m. | #3
I handed over ethtool to John Linville some time ago.

Ben.

On Fri, 2018-12-14 at 17:19 -0800, Maciej Żenczykowski wrote:
> From: Maciej Żenczykowski <maze@google.com>
> 
> prior to fetching it from kernel.
> 
> Otherwise we run the risk of very tail portion of it (dmac field)
> being left entirely uninitialized, and likely containing some sort
> of stale data.
> 
> It seems to likely be some sort of time (a second's counter).
> 
> Tested:
>   'ethtool -c eth1' with old kernel now reports 'dmac: 0' where
>   previously it reported some sort of second counter.
> 
> Signed-off-by: Maciej Żenczykowski <maze@google.com>
> ---
>  ethtool.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/ethtool.c b/ethtool.c
> index 2f7e96bb58db..465eeecb9318 100644
> --- a/ethtool.c
> +++ b/ethtool.c
> @@ -2076,7 +2076,7 @@ static int do_gchannels(struct cmd_context *ctx)
>  
>  static int do_gcoalesce(struct cmd_context *ctx)
>  {
> -	struct ethtool_coalesce ecoal;
> +	struct ethtool_coalesce ecoal = {};
>  	int err;
>  
>  	if (ctx->argc != 0)
John W. Linville Jan. 18, 2019, 6:40 p.m. | #4
On Fri, Dec 14, 2018 at 05:19:23PM -0800, Maciej Żenczykowski wrote:
> From: Maciej Żenczykowski <maze@google.com>
> 
> prior to fetching it from kernel.
> 
> Otherwise we run the risk of very tail portion of it (dmac field)
> being left entirely uninitialized, and likely containing some sort
> of stale data.
> 
> It seems to likely be some sort of time (a second's counter).
> 
> Tested:
>   'ethtool -c eth1' with old kernel now reports 'dmac: 0' where
>   previously it reported some sort of second counter.
> 
> Signed-off-by: Maciej Żenczykowski <maze@google.com>

Queued for next release.

Patch

diff --git a/ethtool.c b/ethtool.c
index 2f7e96bb58db..465eeecb9318 100644
--- a/ethtool.c
+++ b/ethtool.c
@@ -2076,7 +2076,7 @@  static int do_gchannels(struct cmd_context *ctx)
 
 static int do_gcoalesce(struct cmd_context *ctx)
 {
-	struct ethtool_coalesce ecoal;
+	struct ethtool_coalesce ecoal = {};
 	int err;
 
 	if (ctx->argc != 0)