From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752434AbdKVVXV (ORCPT ); Wed, 22 Nov 2017 16:23:21 -0500 Received: from mx2.suse.de ([195.135.220.15]:52309 "EHLO mx2.suse.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751825AbdKVVXS (ORCPT ); Wed, 22 Nov 2017 16:23:18 -0500 Date: Wed, 22 Nov 2017 22:22:58 +0100 From: Borislav Petkov To: Kees Cook Cc: Ricardo Neri , LKML Subject: Re: x86/insn-eval: negative return value? Message-ID: <20171122212258.vfo4xuxvio5dldva@pd.tnic> References: MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: User-Agent: NeoMutt/20170609 (1.8.3) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Wed, Nov 22, 2017 at 12:45:17PM -0800, Kees Cook wrote: > Hi, > > While doing some Clang test builds, this was reported: > > 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)) > ~~~~~~~~^~~~~~~~~~~~~~~~ > > Is this really expected to wrap negative on IA-32e 64-bit mode case? Well, we were saving on structs and crammed two values into one but now that I look at that function, it returns -EINVAL too, which is an int, so its retval should simply be an int. IOW, I guess something like this: --- 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; -- Regards/Gruss, Boris. SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nürnberg) --