diff mbox

x86/sysctl: Fix NULL pointer dereference in error path

Message ID 1484157461-12185-1-git-send-email-andrew.cooper3@citrix.com (mailing list archive)
State New, archived
Headers show

Commit Message

Andrew Cooper Jan. 11, 2017, 5:57 p.m. UTC 
This was introduced by c/s c38869e711 "x86/cpuid: Drop the temporary linear
feature bitmap from struct cpuid_policy", and caught by Coverity.

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
CC: Jan Beulich <JBeulich@suse.com>
---
 xen/arch/x86/sysctl.c | 3 +++
 1 file changed, 3 insertions(+)

Comments

Jan Beulich Jan. 12, 2017, 10:13 a.m. UTC | #1 
>>> On 11.01.17 at 18:57, <andrew.cooper3@citrix.com> wrote:
> --- a/xen/arch/x86/sysctl.c
> +++ b/xen/arch/x86/sysctl.c
> @@ -229,7 +229,10 @@ long arch_do_sysctl(
>  
>          /* Bad featureset index? */
>          if ( !p )
> +        {
>              ret = -EINVAL;
> +            break;
> +        }
>  
>          cpuid_policy_to_featureset(p, featureset);

Considering how the following code is written, adding an "else"
would seem more natural. With the patch above the !ret check
right out of context would then be dead code. With either of
the possible adjustments
Reviewed-by: Jan Beulich <jbeulich@suse.com>

Jan
Andrew Cooper Jan. 12, 2017, 10:26 a.m. UTC | #2 
On 12/01/17 10:13, Jan Beulich wrote:
>>>> On 11.01.17 at 18:57, <andrew.cooper3@citrix.com> wrote:
>> --- a/xen/arch/x86/sysctl.c
>> +++ b/xen/arch/x86/sysctl.c
>> @@ -229,7 +229,10 @@ long arch_do_sysctl(
>>  
>>          /* Bad featureset index? */
>>          if ( !p )
>> +        {
>>              ret = -EINVAL;
>> +            break;
>> +        }
>>  
>>          cpuid_policy_to_featureset(p, featureset);
> Considering how the following code is written, adding an "else"
> would seem more natural. With the patch above the !ret check
> right out of context would then be dead code. With either of
> the possible adjustments
> Reviewed-by: Jan Beulich <jbeulich@suse.com>

Good point - I will make that adjustment.

~Andrew
diff mbox

Patch

diff --git a/xen/arch/x86/sysctl.c b/xen/arch/x86/sysctl.c
index 87da541..274ca0c 100644
--- a/xen/arch/x86/sysctl.c
+++ b/xen/arch/x86/sysctl.c
@@ -229,7 +229,10 @@  long arch_do_sysctl(
 
         /* Bad featureset index? */
         if ( !p )
+        {
             ret = -EINVAL;
+            break;
+        }
 
         cpuid_policy_to_featureset(p, featureset);