* [PATCH 07/11] igb: Avoid uninitialized advertised variable in eee_set_cur
[not found] <1380572952-30729-1-git-send-email-andi@firstfloor.org>
@ 2013-09-30 20:29 ` Andi Kleen
2013-10-01 15:00 ` Wyborny, Carolyn
` (2 more replies)
2013-09-30 20:29 ` [PATCH 10/11] tcp: Always set options to 0 before calling tcp_established_options Andi Kleen
1 sibling, 3 replies; 6+ messages in thread
From: Andi Kleen @ 2013-09-30 20:29 UTC (permalink / raw)
To: linux-kernel; +Cc: Andi Kleen, jeffrey.t.kirsher, netdev
From: Andi Kleen <ak@linux.intel.com>
eee_get_cur assumes that the output data is already zeroed. It can
read-modify-write the advertised field:
if (ipcnfg & E1000_IPCNFG_EEE_100M_AN)
2594 edata->advertised |= ADVERTISED_100baseT_Full;
This is ok for the normal ethtool eee_get call, which always
zeroes the input data before.
But eee_set_cur also calls eee_get_cur and it did not zero the input
field. Later on it then compares agsinst the field, which can contain partial
stack garbage.
Zero the input field in eee_set_cur() too.
Cc: jeffrey.t.kirsher@intel.com
Cc: netdev@vger.kernel.org
Signed-off-by: Andi Kleen <ak@linux.intel.com>
---
drivers/net/ethernet/intel/igb/igb_ethtool.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/net/ethernet/intel/igb/igb_ethtool.c b/drivers/net/ethernet/intel/igb/igb_ethtool.c
index 48cbc83..41e37ff 100644
--- a/drivers/net/ethernet/intel/igb/igb_ethtool.c
+++ b/drivers/net/ethernet/intel/igb/igb_ethtool.c
@@ -2652,6 +2652,8 @@ static int igb_set_eee(struct net_device *netdev,
(hw->phy.media_type != e1000_media_type_copper))
return -EOPNOTSUPP;
+ memset(&eee_curr, 0, sizeof(struct ethtool_eee));
+
ret_val = igb_get_eee(netdev, &eee_curr);
if (ret_val)
return ret_val;
--
1.8.3.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH 10/11] tcp: Always set options to 0 before calling tcp_established_options
[not found] <1380572952-30729-1-git-send-email-andi@firstfloor.org>
2013-09-30 20:29 ` [PATCH 07/11] igb: Avoid uninitialized advertised variable in eee_set_cur Andi Kleen
@ 2013-09-30 20:29 ` Andi Kleen
2013-10-02 20:33 ` David Miller
1 sibling, 1 reply; 6+ messages in thread
From: Andi Kleen @ 2013-09-30 20:29 UTC (permalink / raw)
To: linux-kernel; +Cc: Andi Kleen, netdev
From: Andi Kleen <ak@linux.intel.com>
tcp_established_options assumes opts->options is 0 before calling,
as it read modify writes it.
For the tcp_current_mss() case the opts structure is not zeroed,
so this can be done with uninitialized values.
This is ok, because ->options is not read in this path.
But it's still better to avoid the operation on the uninitialized
field. This shuts up a static code analyzer, and presumably
may help the optimizer.
Cc: netdev@vger.kernel.org
Signed-off-by: Andi Kleen <ak@linux.intel.com>
---
net/ipv4/tcp_output.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/net/ipv4/tcp_output.c b/net/ipv4/tcp_output.c
index 7c83cb8..f3ed78d 100644
--- a/net/ipv4/tcp_output.c
+++ b/net/ipv4/tcp_output.c
@@ -637,6 +637,8 @@ static unsigned int tcp_established_options(struct sock *sk, struct sk_buff *skb
unsigned int size = 0;
unsigned int eff_sacks;
+ opts->options = 0;
+
#ifdef CONFIG_TCP_MD5SIG
*md5 = tp->af_specific->md5_lookup(sk, sk);
if (unlikely(*md5)) {
--
1.8.3.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* RE: [PATCH 07/11] igb: Avoid uninitialized advertised variable in eee_set_cur
2013-09-30 20:29 ` [PATCH 07/11] igb: Avoid uninitialized advertised variable in eee_set_cur Andi Kleen
@ 2013-10-01 15:00 ` Wyborny, Carolyn
2013-10-01 23:10 ` Jeff Kirsher
2013-10-02 20:33 ` David Miller
2 siblings, 0 replies; 6+ messages in thread
From: Wyborny, Carolyn @ 2013-10-01 15:00 UTC (permalink / raw)
To: Andi Kleen, linux-kernel@vger.kernel.org
Cc: Andi Kleen, Kirsher, Jeffrey T, netdev@vger.kernel.org
> -----Original Message-----
> From: netdev-owner@vger.kernel.org [mailto:netdev-owner@vger.kernel.org]
> On Behalf Of Andi Kleen
> Sent: Monday, September 30, 2013 1:29 PM
> To: linux-kernel@vger.kernel.org
> Cc: Andi Kleen; Kirsher, Jeffrey T; netdev@vger.kernel.org
> Subject: [PATCH 07/11] igb: Avoid uninitialized advertised variable in
> eee_set_cur
>
> From: Andi Kleen <ak@linux.intel.com>
>
> eee_get_cur assumes that the output data is already zeroed. It can read-modify-
> write the advertised field:
>
> if (ipcnfg & E1000_IPCNFG_EEE_100M_AN)
> 2594 edata->advertised |= ADVERTISED_100baseT_Full;
>
> This is ok for the normal ethtool eee_get call, which always zeroes the input
> data before.
>
> But eee_set_cur also calls eee_get_cur and it did not zero the input field. Later
> on it then compares agsinst the field, which can contain partial stack garbage.
>
> Zero the input field in eee_set_cur() too.
>
> Cc: jeffrey.t.kirsher@intel.com
> Cc: netdev@vger.kernel.org
> Signed-off-by: Andi Kleen <ak@linux.intel.com>
> ---
> drivers/net/ethernet/intel/igb/igb_ethtool.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/drivers/net/ethernet/intel/igb/igb_ethtool.c
> b/drivers/net/ethernet/intel/igb/igb_ethtool.c
> index 48cbc83..41e37ff 100644
> --- a/drivers/net/ethernet/intel/igb/igb_ethtool.c
> +++ b/drivers/net/ethernet/intel/igb/igb_ethtool.c
> @@ -2652,6 +2652,8 @@ static int igb_set_eee(struct net_device *netdev,
> (hw->phy.media_type != e1000_media_type_copper))
> return -EOPNOTSUPP;
>
> + memset(&eee_curr, 0, sizeof(struct ethtool_eee));
> +
> ret_val = igb_get_eee(netdev, &eee_curr);
> if (ret_val)
> return ret_val;
> --
> 1.8.3.1
>
ACK
Thanks,
Carolyn
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 07/11] igb: Avoid uninitialized advertised variable in eee_set_cur
2013-09-30 20:29 ` [PATCH 07/11] igb: Avoid uninitialized advertised variable in eee_set_cur Andi Kleen
2013-10-01 15:00 ` Wyborny, Carolyn
@ 2013-10-01 23:10 ` Jeff Kirsher
2013-10-02 20:33 ` David Miller
2 siblings, 0 replies; 6+ messages in thread
From: Jeff Kirsher @ 2013-10-01 23:10 UTC (permalink / raw)
To: Andi Kleen; +Cc: linux-kernel, Andi Kleen, netdev
[-- Attachment #1: Type: text/plain, Size: 949 bytes --]
On Mon, 2013-09-30 at 13:29 -0700, Andi Kleen wrote:
> From: Andi Kleen <ak@linux.intel.com>
>
> eee_get_cur assumes that the output data is already zeroed. It can
> read-modify-write the advertised field:
>
> if (ipcnfg & E1000_IPCNFG_EEE_100M_AN)
> 2594 edata->advertised |= ADVERTISED_100baseT_Full;
>
> This is ok for the normal ethtool eee_get call, which always
> zeroes the input data before.
>
> But eee_set_cur also calls eee_get_cur and it did not zero the input
> field. Later on it then compares agsinst the field, which can contain
> partial
> stack garbage.
>
> Zero the input field in eee_set_cur() too.
>
> Cc: jeffrey.t.kirsher@intel.com
> Cc: netdev@vger.kernel.org
> Signed-off-by: Andi Kleen <ak@linux.intel.com>
> ---
> drivers/net/ethernet/intel/igb/igb_ethtool.c | 2 ++
> 1 file changed, 2 insertions(+)
Acked-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 07/11] igb: Avoid uninitialized advertised variable in eee_set_cur
2013-09-30 20:29 ` [PATCH 07/11] igb: Avoid uninitialized advertised variable in eee_set_cur Andi Kleen
2013-10-01 15:00 ` Wyborny, Carolyn
2013-10-01 23:10 ` Jeff Kirsher
@ 2013-10-02 20:33 ` David Miller
2 siblings, 0 replies; 6+ messages in thread
From: David Miller @ 2013-10-02 20:33 UTC (permalink / raw)
To: andi; +Cc: linux-kernel, ak, jeffrey.t.kirsher, netdev
From: Andi Kleen <andi@firstfloor.org>
Date: Mon, 30 Sep 2013 13:29:08 -0700
> From: Andi Kleen <ak@linux.intel.com>
>
> eee_get_cur assumes that the output data is already zeroed. It can
> read-modify-write the advertised field:
>
> if (ipcnfg & E1000_IPCNFG_EEE_100M_AN)
> 2594 edata->advertised |= ADVERTISED_100baseT_Full;
>
> This is ok for the normal ethtool eee_get call, which always
> zeroes the input data before.
>
> But eee_set_cur also calls eee_get_cur and it did not zero the input
> field. Later on it then compares agsinst the field, which can contain partial
> stack garbage.
>
> Zero the input field in eee_set_cur() too.
>
> Cc: jeffrey.t.kirsher@intel.com
> Cc: netdev@vger.kernel.org
> Signed-off-by: Andi Kleen <ak@linux.intel.com>
Applied.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 10/11] tcp: Always set options to 0 before calling tcp_established_options
2013-09-30 20:29 ` [PATCH 10/11] tcp: Always set options to 0 before calling tcp_established_options Andi Kleen
@ 2013-10-02 20:33 ` David Miller
0 siblings, 0 replies; 6+ messages in thread
From: David Miller @ 2013-10-02 20:33 UTC (permalink / raw)
To: andi; +Cc: linux-kernel, ak, netdev
From: Andi Kleen <andi@firstfloor.org>
Date: Mon, 30 Sep 2013 13:29:11 -0700
> From: Andi Kleen <ak@linux.intel.com>
>
> tcp_established_options assumes opts->options is 0 before calling,
> as it read modify writes it.
>
> For the tcp_current_mss() case the opts structure is not zeroed,
> so this can be done with uninitialized values.
>
> This is ok, because ->options is not read in this path.
> But it's still better to avoid the operation on the uninitialized
> field. This shuts up a static code analyzer, and presumably
> may help the optimizer.
>
> Cc: netdev@vger.kernel.org
> Signed-off-by: Andi Kleen <ak@linux.intel.com>
Applied.
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2013-10-02 20:33 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <1380572952-30729-1-git-send-email-andi@firstfloor.org>
2013-09-30 20:29 ` [PATCH 07/11] igb: Avoid uninitialized advertised variable in eee_set_cur Andi Kleen
2013-10-01 15:00 ` Wyborny, Carolyn
2013-10-01 23:10 ` Jeff Kirsher
2013-10-02 20:33 ` David Miller
2013-09-30 20:29 ` [PATCH 10/11] tcp: Always set options to 0 before calling tcp_established_options Andi Kleen
2013-10-02 20:33 ` David Miller
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).