Re: [RFC][PATCH 07/22] x86,extable: Extend extable functionality

[Sean Christopherson <seanjc@google.com>]

On Thu, Nov 04, 2021, Peter Zijlstra wrote:
> +static bool ex_handler_imm_reg(const struct exception_table_entry *fixup,
> +			       struct pt_regs *regs, int reg, int imm)
> +{
> +	*pt_regs_nr(regs, reg) = (long)imm;

This doesn't work for the -EFAULT case because despite being an 'int', @imm is
effectively an 'unsigned short'.  More below.

> +	return ex_handler_default(fixup, regs);
> +}
> +
> +#define EX_TYPE_MASK	0x000000FF
> +#define EX_REG_MASK	0x00000F00
> +#define EX_FLAG_MASK	0x0000F000
> +#define EX_IMM_MASK	0xFFFF0000
> +
>  int ex_get_fixup_type(unsigned long ip)
>  {
>  	const struct exception_table_entry *e = search_exception_tables(ip);
>  
> -	return e ? e->type : EX_TYPE_NONE;
> +	return e ? FIELD_GET(EX_TYPE_MASK, e->type) : EX_TYPE_NONE;
>  }
>  
>  int fixup_exception(struct pt_regs *regs, int trapnr, unsigned long error_code,
>  		    unsigned long fault_addr)
>  {
>  	const struct exception_table_entry *e;
> +	int type, reg, imm;
>  
>  #ifdef CONFIG_PNPBIOS
>  	if (unlikely(SEGMENT_IS_PNP_CODE(regs->cs))) {
> @@ -136,7 +181,11 @@ int fixup_exception(struct pt_regs *regs
>  	if (!e)
>  		return 0;
>  
> -	switch (e->type) {
> +	type = FIELD_GET(EX_TYPE_MASK, e->type);
> +	reg  = FIELD_GET(EX_REG_MASK,  e->type);
> +	imm  = FIELD_GET(EX_IMM_MASK,  e->type);

FIELD_GET casts the result based on the type of the mask, but doesn't explicitly
sign extend the masked field, i.e. there's no intermediate cast to tell the compiler
that the imm is a 16-bit value that should be sign extended.

Modifying FIELD_GET to sign extended is probably a bad idea as I'm guessing the
vast, vast majority of use cases don't want that behavior.  I'm not sure how that
would even work with masks that are e.g. 5 bits or so.

This hack makes things work as expected.

diff --git a/arch/x86/mm/extable.c b/arch/x86/mm/extable.c
index 9dc0685366e5..182d62c11404 100644
--- a/arch/x86/mm/extable.c
+++ b/arch/x86/mm/extable.c
@@ -223,7 +223,7 @@ int fixup_exception(struct pt_regs *regs, int trapnr, unsigned long error_code,

        type = FIELD_GET(EX_TYPE_MASK, e->type);
        reg  = FIELD_GET(EX_REG_MASK,  e->type);
-       imm  = FIELD_GET(EX_IMM_MASK,  e->type);
+       imm  = (int)(short)FIELD_GET(EX_IMM_MASK,  e->type);

        switch (type) {
        case EX_TYPE_DEFAULT:


> +
> +	switch (type) {
>  	case EX_TYPE_DEFAULT:
>  	case EX_TYPE_DEFAULT_MCE_SAFE:
>  		return ex_handler_default(e, regs);
> @@ -165,6 +214,8 @@ int fixup_exception(struct pt_regs *regs
>  		break;
>  	case EX_TYPE_POP_SEG:
>  		return ex_handler_pop_seg(e, regs);
> +	case EX_TYPE_IMM_REG:
> +		return ex_handler_imm_reg(e, regs, reg, imm);
>  	}
>  	BUG();
>  }
> 
>