2017-11-06 13:56:35

by Arnd Bergmann

[permalink] [raw]
Subject: [PATCH 1/3] rtlwifi: fix uninitialized rtlhal->last_suspend_sec time

We set rtlhal->last_suspend_sec to an uninitialized stack variable,
but unfortunately gcc never warned about this, I only found it
while working on another patch. I opened a gcc bug for this.

Presumably the value of rtlhal->last_suspend_sec is not all that
important, but it does get used, so we probably want the
patch backported to stable kernels.

Cc: [email protected]
Link: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=82839
Signed-off-by: Arnd Bergmann <[email protected]>
---
drivers/net/wireless/realtek/rtlwifi/rtl8821ae/hw.c | 1 +
1 file changed, 1 insertion(+)

diff --git a/drivers/net/wireless/realtek/rtlwifi/rtl8821ae/hw.c b/drivers/net/wireless/realtek/rtlwifi/rtl8821ae/hw.c
index 60c82a5b51cd..20ffe856180e 100644
--- a/drivers/net/wireless/realtek/rtlwifi/rtl8821ae/hw.c
+++ b/drivers/net/wireless/realtek/rtlwifi/rtl8821ae/hw.c
@@ -1373,6 +1373,7 @@ static void _rtl8821ae_get_wakeup_reason(struct ieee80211_hw *hw)

ppsc->wakeup_reason = 0;

+ do_gettimeofday(&ts);
rtlhal->last_suspend_sec = ts.tv_sec;

switch (fw_reason) {
--
2.9.0


2017-11-08 12:38:39

by Kalle Valo

[permalink] [raw]
Subject: Re: [1/3] rtlwifi: fix uninitialized rtlhal->last_suspend_sec time

Arnd Bergmann <[email protected]> wrote:

> We set rtlhal->last_suspend_sec to an uninitialized stack variable,
> but unfortunately gcc never warned about this, I only found it
> while working on another patch. I opened a gcc bug for this.
>
> Presumably the value of rtlhal->last_suspend_sec is not all that
> important, but it does get used, so we probably want the
> patch backported to stable kernels.
>
> Cc: [email protected]
> Link: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=82839
> Signed-off-by: Arnd Bergmann <[email protected]>
> Acked-by: Larry Finger <[email protected]>

3 patches applied to wireless-drivers-next.git, thanks.

3f2a162fab15 rtlwifi: fix uninitialized rtlhal->last_suspend_sec time
3c92d5517af8 rtlwifi: use ktime_get_real_seconds() for suspend time
ac978dc79a91 rtlwifi: drop unused ppsc->last_wakeup_time

--
https://patchwork.kernel.org/patch/10043505/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches

2017-11-06 14:22:12

by Larry Finger

[permalink] [raw]
Subject: Re: [PATCH 1/3] rtlwifi: fix uninitialized rtlhal->last_suspend_sec time

On 11/06/2017 07:55 AM, Arnd Bergmann wrote:
> We set rtlhal->last_suspend_sec to an uninitialized stack variable,
> but unfortunately gcc never warned about this, I only found it
> while working on another patch. I opened a gcc bug for this.
>
> Presumably the value of rtlhal->last_suspend_sec is not all that
> important, but it does get used, so we probably want the
> patch backported to stable kernels.
>
> Cc: [email protected]
> Link: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=82839
> Signed-off-by: Arnd Bergmann <[email protected]>

For all three of these patches:

Acked-by: Larry Finger <[email protected]>

Thanks,

Larry

> ---
> drivers/net/wireless/realtek/rtlwifi/rtl8821ae/hw.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/drivers/net/wireless/realtek/rtlwifi/rtl8821ae/hw.c b/drivers/net/wireless/realtek/rtlwifi/rtl8821ae/hw.c
> index 60c82a5b51cd..20ffe856180e 100644
> --- a/drivers/net/wireless/realtek/rtlwifi/rtl8821ae/hw.c
> +++ b/drivers/net/wireless/realtek/rtlwifi/rtl8821ae/hw.c
> @@ -1373,6 +1373,7 @@ static void _rtl8821ae_get_wakeup_reason(struct ieee80211_hw *hw)
>
> ppsc->wakeup_reason = 0;
>
> + do_gettimeofday(&ts);
> rtlhal->last_suspend_sec = ts.tv_sec;
>
> switch (fw_reason) {
>

2017-11-06 13:57:11

by Arnd Bergmann

[permalink] [raw]
Subject: [PATCH 2/3] rtlwifi: use ktime_get_real_seconds() for suspend time

do_gettimeofday() is deprecated and slower than necessary for the purpose
of reading the seconds. This changes rtl_op_suspend/resume to use
ktime_get_real_seconds() instead, which is simpler and avoids confusion
about whether it is y2038-safe or not.

Signed-off-by: Arnd Bergmann <[email protected]>
---
drivers/net/wireless/realtek/rtlwifi/core.c | 10 ++++------
drivers/net/wireless/realtek/rtlwifi/rtl8821ae/hw.c | 3 +--
drivers/net/wireless/realtek/rtlwifi/wifi.h | 2 +-
3 files changed, 6 insertions(+), 9 deletions(-)

diff --git a/drivers/net/wireless/realtek/rtlwifi/core.c b/drivers/net/wireless/realtek/rtlwifi/core.c
index 1147327e6f52..533e9cc4c84b 100644
--- a/drivers/net/wireless/realtek/rtlwifi/core.c
+++ b/drivers/net/wireless/realtek/rtlwifi/core.c
@@ -550,15 +550,13 @@ static int rtl_op_suspend(struct ieee80211_hw *hw,
struct rtl_priv *rtlpriv = rtl_priv(hw);
struct rtl_hal *rtlhal = rtl_hal(rtlpriv);
struct rtl_ps_ctl *ppsc = rtl_psc(rtl_priv(hw));
- struct timeval ts;

RT_TRACE(rtlpriv, COMP_POWER, DBG_DMESG, "\n");
if (WARN_ON(!wow))
return -EINVAL;

/* to resolve s4 can not wake up*/
- do_gettimeofday(&ts);
- rtlhal->last_suspend_sec = ts.tv_sec;
+ rtlhal->last_suspend_sec = ktime_get_real_seconds();

if ((ppsc->wo_wlan_mode & WAKE_ON_PATTERN_MATCH) && wow->n_patterns)
_rtl_add_wowlan_patterns(hw, wow);
@@ -577,7 +575,7 @@ static int rtl_op_resume(struct ieee80211_hw *hw)
struct rtl_priv *rtlpriv = rtl_priv(hw);
struct rtl_hal *rtlhal = rtl_hal(rtlpriv);
struct rtl_mac *mac = rtl_mac(rtl_priv(hw));
- struct timeval ts;
+ time64_t now;

RT_TRACE(rtlpriv, COMP_POWER, DBG_DMESG, "\n");
rtlhal->driver_is_goingto_unload = false;
@@ -585,8 +583,8 @@ static int rtl_op_resume(struct ieee80211_hw *hw)
rtlhal->wake_from_pnp_sleep = true;

/* to resovle s4 can not wake up*/
- do_gettimeofday(&ts);
- if (ts.tv_sec - rtlhal->last_suspend_sec < 5)
+ now = ktime_get_real_seconds();
+ if (now - rtlhal->last_suspend_sec < 5)
return -1;

rtl_op_start(hw);
diff --git a/drivers/net/wireless/realtek/rtlwifi/rtl8821ae/hw.c b/drivers/net/wireless/realtek/rtlwifi/rtl8821ae/hw.c
index 20ffe856180e..ad28e188bd04 100644
--- a/drivers/net/wireless/realtek/rtlwifi/rtl8821ae/hw.c
+++ b/drivers/net/wireless/realtek/rtlwifi/rtl8821ae/hw.c
@@ -1373,8 +1373,7 @@ static void _rtl8821ae_get_wakeup_reason(struct ieee80211_hw *hw)

ppsc->wakeup_reason = 0;

- do_gettimeofday(&ts);
- rtlhal->last_suspend_sec = ts.tv_sec;
+ rtlhal->last_suspend_sec = ktime_get_real_seconds();

switch (fw_reason) {
case FW_WOW_V2_PTK_UPDATE_EVENT:
diff --git a/drivers/net/wireless/realtek/rtlwifi/wifi.h b/drivers/net/wireless/realtek/rtlwifi/wifi.h
index 22afc14c3da6..6f1b0f4667d9 100644
--- a/drivers/net/wireless/realtek/rtlwifi/wifi.h
+++ b/drivers/net/wireless/realtek/rtlwifi/wifi.h
@@ -1599,7 +1599,7 @@ struct rtl_hal {
bool enter_pnp_sleep;
bool wake_from_pnp_sleep;
bool wow_enabled;
- __kernel_time_t last_suspend_sec;
+ time64_t last_suspend_sec;
u32 wowlan_fwsize;
u8 *wowlan_firmware;

--
2.9.0

2017-11-06 13:57:57

by Arnd Bergmann

[permalink] [raw]
Subject: [PATCH 3/3] rtlwifi: drop unused ppsc->last_wakeup_time

The calculation of ppsc->last_wakeup_time is not y2038-safe, but
the variable is not used at all, so we can simply drop it.

Signed-off-by: Arnd Bergmann <[email protected]>
---
drivers/net/wireless/realtek/rtlwifi/rtl8821ae/hw.c | 5 -----
drivers/net/wireless/realtek/rtlwifi/wifi.h | 2 --
2 files changed, 7 deletions(-)

diff --git a/drivers/net/wireless/realtek/rtlwifi/rtl8821ae/hw.c b/drivers/net/wireless/realtek/rtlwifi/rtl8821ae/hw.c
index ad28e188bd04..36f785c4ea0f 100644
--- a/drivers/net/wireless/realtek/rtlwifi/rtl8821ae/hw.c
+++ b/drivers/net/wireless/realtek/rtlwifi/rtl8821ae/hw.c
@@ -1364,7 +1364,6 @@ static void _rtl8821ae_get_wakeup_reason(struct ieee80211_hw *hw)
struct rtl_hal *rtlhal = rtl_hal(rtl_priv(hw));
struct rtl_ps_ctl *ppsc = rtl_psc(rtlpriv);
u8 fw_reason = 0;
- struct timeval ts;

fw_reason = rtl_read_byte(rtlpriv, REG_MCUTST_WOWLAN);

@@ -1378,15 +1377,11 @@ static void _rtl8821ae_get_wakeup_reason(struct ieee80211_hw *hw)
switch (fw_reason) {
case FW_WOW_V2_PTK_UPDATE_EVENT:
ppsc->wakeup_reason = WOL_REASON_PTK_UPDATE;
- do_gettimeofday(&ts);
- ppsc->last_wakeup_time = ts.tv_sec*1000 + ts.tv_usec/1000;
RT_TRACE(rtlpriv, COMP_POWER, DBG_DMESG,
"It's a WOL PTK Key update event!\n");
break;
case FW_WOW_V2_GTK_UPDATE_EVENT:
ppsc->wakeup_reason = WOL_REASON_GTK_UPDATE;
- do_gettimeofday(&ts);
- ppsc->last_wakeup_time = ts.tv_sec*1000 + ts.tv_usec/1000;
RT_TRACE(rtlpriv, COMP_POWER, DBG_DMESG,
"It's a WOL GTK Key update event!\n");
break;
diff --git a/drivers/net/wireless/realtek/rtlwifi/wifi.h b/drivers/net/wireless/realtek/rtlwifi/wifi.h
index 6f1b0f4667d9..d9776af41976 100644
--- a/drivers/net/wireless/realtek/rtlwifi/wifi.h
+++ b/drivers/net/wireless/realtek/rtlwifi/wifi.h
@@ -1953,8 +1953,6 @@ struct rtl_ps_ctl {
u8 gtk_offload_enable;
/* Used for WOL, indicates the reason for waking event.*/
u32 wakeup_reason;
- /* Record the last waking time for comparison with setting key. */
- u64 last_wakeup_time;
};

struct rtl_stats {
--
2.9.0