diff mbox

[v2,1/4] x86emul: always fill x86_insn_modrm()'s outputs

Message ID 58F606C702000078001517CA@prv-mh.provo.novell.com (mailing list archive)
State New, archived
Headers show

Commit Message

Jan Beulich April 18, 2017, 10:29 a.m. UTC 
The function is rather unlikely to be called for insns which don't have
ModRM bytes, and hence addressing Coverity's recurring complaint of
callers potentially consuming uninitialized data when they know that
certain opcodes have ModRM bytes can be suppressed this way without
unduly adding overhead to fast paths.

Signed-off-by: Jan Beulich <jbeulich@suse.com>
x86emul: always fill x86_insn_modrm()'s outputs

The function is rather unlikely to be called for insns which don't have
ModRM bytes, and hence addressing Coverity's recurring complaint of
callers potentially consuming uninitialized data when they know that
certain opcodes have ModRM bytes can be suppressed this way without
unduly adding overhead to fast paths.

Signed-off-by: Jan Beulich <jbeulich@suse.com>

--- a/xen/arch/x86/x86_emulate/x86_emulate.c
+++ b/xen/arch/x86/x86_emulate/x86_emulate.c
@@ -8017,8 +8017,14 @@ x86_insn_modrm(const struct x86_emulate_
 {
     check_state(state);
 
-    if ( state->modrm_mod > 3 )
+    if ( unlikely(state->modrm_mod > 3) )
+    {
+        if ( rm )
+            *rm = ~0U;
+        if ( reg )
+            *reg = ~0U;
         return -EINVAL;
+    }
 
     if ( rm )
         *rm = state->modrm_rm;

Comments

Andrew Cooper April 18, 2017, 10:33 a.m. UTC | #1 
On 18/04/17 11:29, Jan Beulich wrote:
> The function is rather unlikely to be called for insns which don't have
> ModRM bytes, and hence addressing Coverity's recurring complaint of
> callers potentially consuming uninitialized data when they know that
> certain opcodes have ModRM bytes can be suppressed this way without
> unduly adding overhead to fast paths.
>
> Signed-off-by: Jan Beulich <jbeulich@suse.com>

Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>

>
> --- a/xen/arch/x86/x86_emulate/x86_emulate.c
> +++ b/xen/arch/x86/x86_emulate/x86_emulate.c
> @@ -8017,8 +8017,14 @@ x86_insn_modrm(const struct x86_emulate_
>  {
>      check_state(state);
>  
> -    if ( state->modrm_mod > 3 )
> +    if ( unlikely(state->modrm_mod > 3) )
> +    {
> +        if ( rm )
> +            *rm = ~0U;
> +        if ( reg )
> +            *reg = ~0U;
>          return -EINVAL;
> +    }
>  
>      if ( rm )
>          *rm = state->modrm_rm;
>
>
>
diff mbox

Patch

--- a/xen/arch/x86/x86_emulate/x86_emulate.c
+++ b/xen/arch/x86/x86_emulate/x86_emulate.c
@@ -8017,8 +8017,14 @@  x86_insn_modrm(const struct x86_emulate_
 {
     check_state(state);
 
-    if ( state->modrm_mod > 3 )
+    if ( unlikely(state->modrm_mod > 3) )
+    {
+        if ( rm )
+            *rm = ~0U;
+        if ( reg )
+            *reg = ~0U;
         return -EINVAL;
+    }
 
     if ( rm )
         *rm = state->modrm_rm;