All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] x86/umip: Fix insn_get_code_seg_params()'s return value
@ 2017-11-23  9:19 Borislav Petkov
  2017-11-23 19:25 ` [tip:x86/urgent] " tip-bot for Borislav Petkov
  2017-11-28  3:56 ` [PATCH] " Ricardo Neri
  0 siblings, 2 replies; 3+ messages in thread
From: Borislav Petkov @ 2017-11-23  9:19 UTC (permalink / raw)
  To: X86 ML; +Cc: LKML, Kees Cook, Nick Desaulniers, ricardo.neri-calderon

From: Borislav Petkov <bp@suse.de>

In order to save on redundant structs definitions
insn_get_code_seg_params() was made to return two 4-bit values in a char
but clang complains:

  arch/x86/lib/insn-eval.c:780:10: warning: implicit conversion from 'int' to 'char'
	  changes value from 132 to -124 [-Wconstant-conversion]
                  return INSN_CODE_SEG_PARAMS(4, 8);
                  ~~~~~~ ^~~~~~~~~~~~~~~~~~~~~~~~~~
  ./arch/x86/include/asm/insn-eval.h:16:57: note: expanded from macro 'INSN_CODE_SEG_PARAMS'
  #define INSN_CODE_SEG_PARAMS(oper_sz, addr_sz) (oper_sz | (addr_sz << 4))

Those two values do get picked apart afterwards the opposite way of how
they were ORed so wrt to the LSByte, the return value is the same.

But this function returns -EINVAL in the error case, which is an int. So
make it return an int which is the native word size anyway and thus fix
the clang warning.

Signed-off-by: Borislav Petkov <bp@suse.de>
Reported-by: Kees Cook <keescook@google.com>
Reported-by: Nick Desaulniers <nick.desaulniers@gmail.com>
Cc: ricardo.neri-calderon@linux.intel.com
---
 arch/x86/include/asm/insn-eval.h | 2 +-
 arch/x86/kernel/umip.c           | 2 +-
 arch/x86/lib/insn-eval.c         | 4 ++--
 3 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/arch/x86/include/asm/insn-eval.h b/arch/x86/include/asm/insn-eval.h
index e1d3b4ce8a92..2b6ccf2c49f1 100644
--- a/arch/x86/include/asm/insn-eval.h
+++ b/arch/x86/include/asm/insn-eval.h
@@ -18,6 +18,6 @@
 void __user *insn_get_addr_ref(struct insn *insn, struct pt_regs *regs);
 int insn_get_modrm_rm_off(struct insn *insn, struct pt_regs *regs);
 unsigned long insn_get_seg_base(struct pt_regs *regs, int seg_reg_idx);
-char insn_get_code_seg_params(struct pt_regs *regs);
+int insn_get_code_seg_params(struct pt_regs *regs);
 
 #endif /* _ASM_X86_INSN_EVAL_H */
diff --git a/arch/x86/kernel/umip.c b/arch/x86/kernel/umip.c
index dabbac30acdf..f44ce0fb3583 100644
--- a/arch/x86/kernel/umip.c
+++ b/arch/x86/kernel/umip.c
@@ -319,7 +319,7 @@ bool fixup_umip_exception(struct pt_regs *regs)
 	unsigned char buf[MAX_INSN_SIZE];
 	void __user *uaddr;
 	struct insn insn;
-	char seg_defs;
+	int seg_defs;
 
 	if (!regs)
 		return false;
diff --git a/arch/x86/lib/insn-eval.c b/arch/x86/lib/insn-eval.c
index 35625d279458..9119d8e41f1f 100644
--- a/arch/x86/lib/insn-eval.c
+++ b/arch/x86/lib/insn-eval.c
@@ -733,11 +733,11 @@ static unsigned long get_seg_limit(struct pt_regs *regs, int seg_reg_idx)
  *
  * Returns:
  *
- * A signed 8-bit value containing the default parameters on success.
+ * An int containing ORed-in default parameters on success.
  *
  * -EINVAL on error.
  */
-char insn_get_code_seg_params(struct pt_regs *regs)
+int insn_get_code_seg_params(struct pt_regs *regs)
 {
 	struct desc_struct *desc;
 	short sel;
-- 
2.13.0

^ permalink raw reply related	[flat|nested] 3+ messages in thread

* [tip:x86/urgent] x86/umip: Fix insn_get_code_seg_params()'s return value
  2017-11-23  9:19 [PATCH] x86/umip: Fix insn_get_code_seg_params()'s return value Borislav Petkov
@ 2017-11-23 19:25 ` tip-bot for Borislav Petkov
  2017-11-28  3:56 ` [PATCH] " Ricardo Neri
  1 sibling, 0 replies; 3+ messages in thread
From: tip-bot for Borislav Petkov @ 2017-11-23 19:25 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: bp, keescook, mingo, hpa, tglx, nick.desaulniers, linux-kernel

Commit-ID:  e2a5dca753d1cdc3212519023ed8a13e13f5495b
Gitweb:     https://git.kernel.org/tip/e2a5dca753d1cdc3212519023ed8a13e13f5495b
Author:     Borislav Petkov <bp@suse.de>
AuthorDate: Thu, 23 Nov 2017 10:19:51 +0100
Committer:  Thomas Gleixner <tglx@linutronix.de>
CommitDate: Thu, 23 Nov 2017 20:17:59 +0100

x86/umip: Fix insn_get_code_seg_params()'s return value

In order to save on redundant structs definitions
insn_get_code_seg_params() was made to return two 4-bit values in a char
but clang complains:

  arch/x86/lib/insn-eval.c:780:10: warning: implicit conversion from 'int' to 'char'
	  changes value from 132 to -124 [-Wconstant-conversion]
                  return INSN_CODE_SEG_PARAMS(4, 8);
                  ~~~~~~ ^~~~~~~~~~~~~~~~~~~~~~~~~~
  ./arch/x86/include/asm/insn-eval.h:16:57: note: expanded from macro 'INSN_CODE_SEG_PARAMS'
  #define INSN_CODE_SEG_PARAMS(oper_sz, addr_sz) (oper_sz | (addr_sz << 4))

Those two values do get picked apart afterwards the opposite way of how
they were ORed so wrt to the LSByte, the return value is the same.

But this function returns -EINVAL in the error case, which is an int. So
make it return an int which is the native word size anyway and thus fix
the clang warning.

Reported-by: Kees Cook <keescook@google.com>
Reported-by: Nick Desaulniers <nick.desaulniers@gmail.com>
Signed-off-by: Borislav Petkov <bp@suse.de>
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Cc: ricardo.neri-calderon@linux.intel.com
Link: https://lkml.kernel.org/r/20171123091951.1462-1-bp@alien8.de

---
 arch/x86/include/asm/insn-eval.h | 2 +-
 arch/x86/kernel/umip.c           | 2 +-
 arch/x86/lib/insn-eval.c         | 4 ++--
 3 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/arch/x86/include/asm/insn-eval.h b/arch/x86/include/asm/insn-eval.h
index e1d3b4c..2b6ccf2 100644
--- a/arch/x86/include/asm/insn-eval.h
+++ b/arch/x86/include/asm/insn-eval.h
@@ -18,6 +18,6 @@
 void __user *insn_get_addr_ref(struct insn *insn, struct pt_regs *regs);
 int insn_get_modrm_rm_off(struct insn *insn, struct pt_regs *regs);
 unsigned long insn_get_seg_base(struct pt_regs *regs, int seg_reg_idx);
-char insn_get_code_seg_params(struct pt_regs *regs);
+int insn_get_code_seg_params(struct pt_regs *regs);
 
 #endif /* _ASM_X86_INSN_EVAL_H */
diff --git a/arch/x86/kernel/umip.c b/arch/x86/kernel/umip.c
index dabbac3..f44ce0f 100644
--- a/arch/x86/kernel/umip.c
+++ b/arch/x86/kernel/umip.c
@@ -319,7 +319,7 @@ bool fixup_umip_exception(struct pt_regs *regs)
 	unsigned char buf[MAX_INSN_SIZE];
 	void __user *uaddr;
 	struct insn insn;
-	char seg_defs;
+	int seg_defs;
 
 	if (!regs)
 		return false;
diff --git a/arch/x86/lib/insn-eval.c b/arch/x86/lib/insn-eval.c
index 35625d2..9119d8e 100644
--- a/arch/x86/lib/insn-eval.c
+++ b/arch/x86/lib/insn-eval.c
@@ -733,11 +733,11 @@ static unsigned long get_seg_limit(struct pt_regs *regs, int seg_reg_idx)
  *
  * Returns:
  *
- * A signed 8-bit value containing the default parameters on success.
+ * An int containing ORed-in default parameters on success.
  *
  * -EINVAL on error.
  */
-char insn_get_code_seg_params(struct pt_regs *regs)
+int insn_get_code_seg_params(struct pt_regs *regs)
 {
 	struct desc_struct *desc;
 	short sel;

^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH] x86/umip: Fix insn_get_code_seg_params()'s return value
  2017-11-23  9:19 [PATCH] x86/umip: Fix insn_get_code_seg_params()'s return value Borislav Petkov
  2017-11-23 19:25 ` [tip:x86/urgent] " tip-bot for Borislav Petkov
@ 2017-11-28  3:56 ` Ricardo Neri
  1 sibling, 0 replies; 3+ messages in thread
From: Ricardo Neri @ 2017-11-28  3:56 UTC (permalink / raw)
  To: Borislav Petkov; +Cc: X86 ML, LKML, Kees Cook, Nick Desaulniers

On Thu, Nov 23, 2017 at 10:19:51AM +0100, Borislav Petkov wrote:
> From: Borislav Petkov <bp@suse.de>
> 
> In order to save on redundant structs definitions
> insn_get_code_seg_params() was made to return two 4-bit values in a char
> but clang complains:
> 
>   arch/x86/lib/insn-eval.c:780:10: warning: implicit conversion from 'int' to 'char'
> 	  changes value from 132 to -124 [-Wconstant-conversion]
>                   return INSN_CODE_SEG_PARAMS(4, 8);
>                   ~~~~~~ ^~~~~~~~~~~~~~~~~~~~~~~~~~
>   ./arch/x86/include/asm/insn-eval.h:16:57: note: expanded from macro 'INSN_CODE_SEG_PARAMS'
>   #define INSN_CODE_SEG_PARAMS(oper_sz, addr_sz) (oper_sz | (addr_sz << 4))
> 
> Those two values do get picked apart afterwards the opposite way of how
> they were ORed so wrt to the LSByte, the return value is the same.
> 
> But this function returns -EINVAL in the error case, which is an int. So
> make it return an int which is the native word size anyway and thus fix
> the clang warning.
> 
> Signed-off-by: Borislav Petkov <bp@suse.de>
> Reported-by: Kees Cook <keescook@google.com>
> Reported-by: Nick Desaulniers <nick.desaulniers@gmail.com>
> Cc: ricardo.neri-calderon@linux.intel.com

Thanks Kees and Nick for finding this bug. Thanks Borislav for the quick fix!
This change looks OK to me.

BR,
Ricardo

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2017-11-28  3:57 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-11-23  9:19 [PATCH] x86/umip: Fix insn_get_code_seg_params()'s return value Borislav Petkov
2017-11-23 19:25 ` [tip:x86/urgent] " tip-bot for Borislav Petkov
2017-11-28  3:56 ` [PATCH] " Ricardo Neri

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.