Re: [PATCH 22/28] x86: apm: avoid uninitialized data

[Luis R. Rodriguez]

On Tue, Oct 18, 2016 at 12:16:10AM +0200, Arnd Bergmann wrote:
> apm_bios_call() can fail, and return a status in its argument
> structure. If that status however is zero during a call from
> apm_get_power_status(), we end up using data that may have
> never been set, as reported by "gcc -Wmaybe-uninitialized":

Userspace *may* already rely on this broken behavior for broken
BIOSes which may leave the return value as 0, ignoring that,
this change makes sense to me given that handling the error
would be better than relying any possible invalid data.

> arch/x86/kernel/apm_32.c: In function ‘apm’:
> arch/x86/kernel/apm_32.c:1729:17: error: ‘bx’ may be used uninitialized in 
> this function [-Werror=maybe-uninitialized]
> arch/x86/kernel/apm_32.c:1835:5: error: ‘cx’ may be used uninitialized in 
> this function [-Werror=maybe-uninitialized]
> arch/x86/kernel/apm_32.c:1730:17: note: ‘cx’ was declared here
> arch/x86/kernel/apm_32.c:1842:27: error: ‘dx’ may be used uninitialized in 
> this function [-Werror=maybe-uninitialized]
> arch/x86/kernel/apm_32.c:1731:17: note: ‘dx’ was declared here
> 
> This changes the function to return "APM_NO_ERROR" here, which
> makes the code more robust to broken BIOS versions, and avoids
> the warning.
> 
> Cc: x...@kernel.org
> Signed-off-by: Arnd Bergmann 

Reviewed-by: Luis R. Rodriguez 

  Luis

> ---
>  arch/x86/kernel/apm_32.c | 5 -
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/x86/kernel/apm_32.c b/arch/x86/kernel/apm_32.c
> index c7364bd..51287cd 100644
> --- a/arch/x86/kernel/apm_32.c
> +++ b/arch/x86/kernel/apm_32.c
> @@ -1042,8 +1042,11 @@ static int apm_get_power_status(u_short *status, 
> u_short *bat, u_short *life)
>  
>   if (apm_info.get_power_status_broken)
>   return APM_32_UNSUPPORTED;
> - if (apm_bios_call())
> + if (apm_bios_call()) {
> + if (!call.err)
> + return APM_NO_ERROR;
>   return call.err;
> + }
>   *status = call.ebx;
>   *bat = call.ecx;
>   if (apm_info.get_power_status_swabinminutes) {
> -- 
> 2.9.0
> 
> 

-- 
Luis Rodriguez, SUSE LINUX GmbH
Maxfeldstrasse 5; D-90409 Nuernberg

Re: [PATCH 22/28] x86: apm: avoid uninitialized data

[Luis R. Rodriguez]

On Tue, Oct 18, 2016 at 12:16:10AM +0200, Arnd Bergmann wrote:
> apm_bios_call() can fail, and return a status in its argument
> structure. If that status however is zero during a call from
> apm_get_power_status(), we end up using data that may have
> never been set, as reported by "gcc -Wmaybe-uninitialized":

Userspace *may* already rely on this broken behavior for broken
BIOSes which may leave the return value as 0, ignoring that,
this change makes sense to me given that handling the error
would be better than relying any possible invalid data.

> arch/x86/kernel/apm_32.c: In function ‘apm’:
> arch/x86/kernel/apm_32.c:1729:17: error: ‘bx’ may be used uninitialized in 
> this function [-Werror=maybe-uninitialized]
> arch/x86/kernel/apm_32.c:1835:5: error: ‘cx’ may be used uninitialized in 
> this function [-Werror=maybe-uninitialized]
> arch/x86/kernel/apm_32.c:1730:17: note: ‘cx’ was declared here
> arch/x86/kernel/apm_32.c:1842:27: error: ‘dx’ may be used uninitialized in 
> this function [-Werror=maybe-uninitialized]
> arch/x86/kernel/apm_32.c:1731:17: note: ‘dx’ was declared here
> 
> This changes the function to return "APM_NO_ERROR" here, which
> makes the code more robust to broken BIOS versions, and avoids
> the warning.
> 
> Cc: x...@kernel.org
> Signed-off-by: Arnd Bergmann 

Reviewed-by: Luis R. Rodriguez 

  Luis

> ---
>  arch/x86/kernel/apm_32.c | 5 -
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/x86/kernel/apm_32.c b/arch/x86/kernel/apm_32.c
> index c7364bd..51287cd 100644
> --- a/arch/x86/kernel/apm_32.c
> +++ b/arch/x86/kernel/apm_32.c
> @@ -1042,8 +1042,11 @@ static int apm_get_power_status(u_short *status, 
> u_short *bat, u_short *life)
>  
>   if (apm_info.get_power_status_broken)
>   return APM_32_UNSUPPORTED;
> - if (apm_bios_call())
> + if (apm_bios_call()) {
> + if (!call.err)
> + return APM_NO_ERROR;
>   return call.err;
> + }
>   *status = call.ebx;
>   *bat = call.ecx;
>   if (apm_info.get_power_status_swabinminutes) {
> -- 
> 2.9.0
> 
> 

-- 
Luis Rodriguez, SUSE LINUX GmbH
Maxfeldstrasse 5; D-90409 Nuernberg

Re: [PATCH 22/28] x86: apm: avoid uninitialized data

[Jiri Kosina]

On Tue, 18 Oct 2016, Arnd Bergmann wrote:

> apm_bios_call() can fail, and return a status in its argument
> structure. If that status however is zero during a call from
> apm_get_power_status(), we end up using data that may have
> never been set, as reported by "gcc -Wmaybe-uninitialized":
> 
> arch/x86/kernel/apm_32.c: In function ‘apm’:
> arch/x86/kernel/apm_32.c:1729:17: error: ‘bx’ may be used uninitialized in 
> this function [-Werror=maybe-uninitialized]
> arch/x86/kernel/apm_32.c:1835:5: error: ‘cx’ may be used uninitialized in 
> this function [-Werror=maybe-uninitialized]
> arch/x86/kernel/apm_32.c:1730:17: note: ‘cx’ was declared here
> arch/x86/kernel/apm_32.c:1842:27: error: ‘dx’ may be used uninitialized in 
> this function [-Werror=maybe-uninitialized]
> arch/x86/kernel/apm_32.c:1731:17: note: ‘dx’ was declared here
> 
> This changes the function to return "APM_NO_ERROR" here, which
> makes the code more robust to broken BIOS versions, and avoids
> the warning.
> 
> Cc: x...@kernel.org
> Signed-off-by: Arnd Bergmann 

Makes sense.

Reviewed-by: Jiri Kosina 

Thanks,

-- 
Jiri Kosina
SUSE Labs

Re: [PATCH 22/28] x86: apm: avoid uninitialized data

[Jiri Kosina]

On Tue, 18 Oct 2016, Arnd Bergmann wrote:

> apm_bios_call() can fail, and return a status in its argument
> structure. If that status however is zero during a call from
> apm_get_power_status(), we end up using data that may have
> never been set, as reported by "gcc -Wmaybe-uninitialized":
> 
> arch/x86/kernel/apm_32.c: In function ‘apm’:
> arch/x86/kernel/apm_32.c:1729:17: error: ‘bx’ may be used uninitialized in 
> this function [-Werror=maybe-uninitialized]
> arch/x86/kernel/apm_32.c:1835:5: error: ‘cx’ may be used uninitialized in 
> this function [-Werror=maybe-uninitialized]
> arch/x86/kernel/apm_32.c:1730:17: note: ‘cx’ was declared here
> arch/x86/kernel/apm_32.c:1842:27: error: ‘dx’ may be used uninitialized in 
> this function [-Werror=maybe-uninitialized]
> arch/x86/kernel/apm_32.c:1731:17: note: ‘dx’ was declared here
> 
> This changes the function to return "APM_NO_ERROR" here, which
> makes the code more robust to broken BIOS versions, and avoids
> the warning.
> 
> Cc: x...@kernel.org
> Signed-off-by: Arnd Bergmann 

Makes sense.

Reviewed-by: Jiri Kosina 

Thanks,

-- 
Jiri Kosina
SUSE Labs

[PATCH 22/28] x86: apm: avoid uninitialized data

[Arnd Bergmann]

apm_bios_call() can fail, and return a status in its argument
structure. If that status however is zero during a call from
apm_get_power_status(), we end up using data that may have
never been set, as reported by "gcc -Wmaybe-uninitialized":

arch/x86/kernel/apm_32.c: In function ‘apm’:
arch/x86/kernel/apm_32.c:1729:17: error: ‘bx’ may be used uninitialized in this 
function [-Werror=maybe-uninitialized]
arch/x86/kernel/apm_32.c:1835:5: error: ‘cx’ may be used uninitialized in this 
function [-Werror=maybe-uninitialized]
arch/x86/kernel/apm_32.c:1730:17: note: ‘cx’ was declared here
arch/x86/kernel/apm_32.c:1842:27: error: ‘dx’ may be used uninitialized in this 
function [-Werror=maybe-uninitialized]
arch/x86/kernel/apm_32.c:1731:17: note: ‘dx’ was declared here

This changes the function to return "APM_NO_ERROR" here, which
makes the code more robust to broken BIOS versions, and avoids
the warning.

Cc: x...@kernel.org
Signed-off-by: Arnd Bergmann 
---
 arch/x86/kernel/apm_32.c | 5 -
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/arch/x86/kernel/apm_32.c b/arch/x86/kernel/apm_32.c
index c7364bd..51287cd 100644
--- a/arch/x86/kernel/apm_32.c
+++ b/arch/x86/kernel/apm_32.c
@@ -1042,8 +1042,11 @@ static int apm_get_power_status(u_short *status, u_short 
*bat, u_short *life)
 
if (apm_info.get_power_status_broken)
return APM_32_UNSUPPORTED;
-   if (apm_bios_call())
+   if (apm_bios_call()) {
+   if (!call.err)
+   return APM_NO_ERROR;
return call.err;
+   }
*status = call.ebx;
*bat = call.ecx;
if (apm_info.get_power_status_swabinminutes) {
-- 
2.9.0

[PATCH 22/28] x86: apm: avoid uninitialized data

[Arnd Bergmann]

apm_bios_call() can fail, and return a status in its argument
structure. If that status however is zero during a call from
apm_get_power_status(), we end up using data that may have
never been set, as reported by "gcc -Wmaybe-uninitialized":

arch/x86/kernel/apm_32.c: In function ‘apm’:
arch/x86/kernel/apm_32.c:1729:17: error: ‘bx’ may be used uninitialized in this 
function [-Werror=maybe-uninitialized]
arch/x86/kernel/apm_32.c:1835:5: error: ‘cx’ may be used uninitialized in this 
function [-Werror=maybe-uninitialized]
arch/x86/kernel/apm_32.c:1730:17: note: ‘cx’ was declared here
arch/x86/kernel/apm_32.c:1842:27: error: ‘dx’ may be used uninitialized in this 
function [-Werror=maybe-uninitialized]
arch/x86/kernel/apm_32.c:1731:17: note: ‘dx’ was declared here

This changes the function to return "APM_NO_ERROR" here, which
makes the code more robust to broken BIOS versions, and avoids
the warning.

Cc: x...@kernel.org
Signed-off-by: Arnd Bergmann 
---
 arch/x86/kernel/apm_32.c | 5 -
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/arch/x86/kernel/apm_32.c b/arch/x86/kernel/apm_32.c
index c7364bd..51287cd 100644
--- a/arch/x86/kernel/apm_32.c
+++ b/arch/x86/kernel/apm_32.c
@@ -1042,8 +1042,11 @@ static int apm_get_power_status(u_short *status, u_short 
*bat, u_short *life)
 
if (apm_info.get_power_status_broken)
return APM_32_UNSUPPORTED;
-   if (apm_bios_call())
+   if (apm_bios_call()) {
+   if (!call.err)
+   return APM_NO_ERROR;
return call.err;
+   }
*status = call.ebx;
*bat = call.ecx;
if (apm_info.get_power_status_swabinminutes) {
-- 
2.9.0