[trusty/master-next 1/1] UBUNTU: SAUCE: x86, extable: fix uaccess fixup detection

Stefan Bader stefan.bader at canonical.com
Fri Feb 23 09:04:01 UTC 2018


On 22.02.2018 11:24, Andy Whitcroft wrote:
> The existing code intends to identify a subset of fixups which need
> special handling, uaccess related faults need to record the failure.
> This is done by adjusting the fixup code pointer by a (random) constant
> 0x7ffffff0.  This is detected in fixup_exception by comparing the two
> pointers.  The intent of this code is to detect the the delta between
> the original code and its fixup code being greater than the constant.
> However, the code as written triggers undefined comparison behaviour.
> In this kernel this prevents the condition triggering, leading to panics
> when jumping to the corrupted fixup address.
> 
> Convert the code to better implement the intent.  Convert both of the
> offsets to final addresses and compare the delta between those.  Also add
> a massive comment to explain all of this including the implicit assumptions
> on order of the segments that this comparison implies.
> 
> Fixes: 706276543b69 ("x86, extable: Switch to relative exception table entries")
> Signed-off-by: Andy Whitcroft <apw at canonical.com>
> ---
>  arch/x86/mm/extable.c | 16 ++++++++++++++--
>  1 file changed, 14 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/x86/mm/extable.c b/arch/x86/mm/extable.c
> index 903ec1e9c326..a06be2f7f1bb 100644
> --- a/arch/x86/mm/extable.c
> +++ b/arch/x86/mm/extable.c
> @@ -17,6 +17,7 @@ ex_fixup_addr(const struct exception_table_entry *x)
>  int fixup_exception(struct pt_regs *regs)
>  {
>  	const struct exception_table_entry *fixup;
> +	unsigned long insn_ip;
>  	unsigned long new_ip;
>  
>  #ifdef CONFIG_PNPBIOS
> @@ -35,9 +36,17 @@ int fixup_exception(struct pt_regs *regs)
>  
>  	fixup = search_exception_tables(regs->ip);
>  	if (fixup) {
> +		insn_ip = ex_insn_addr(fixup);
>  		new_ip = ex_fixup_addr(fixup);
>  
> -		if (fixup->fixup - fixup->insn >= 0x7ffffff0 - 4) {

This is probably deliberate but I am just wondering why the -4 gets dropped here...

-Stefan
> +		/*
> +		 * If the code and its fixup are "very far apart" then
> +		 * they are infact tagged as uaccess'es.  Handle them
> +		 * specially and fix the fixup address.  This relies on
> +		 * the .fixup section being at higher addresses that the
> +		 * original code.
> +		 */
> +		if (new_ip - insn_ip >= 0x7ffffff0) {
>  			/* Special hack for uaccess_err */
>  			current_thread_info()->uaccess_err = 1;
>  			new_ip -= 0x7ffffff0;
> @@ -53,13 +62,16 @@ int fixup_exception(struct pt_regs *regs)
>  int __init early_fixup_exception(unsigned long *ip)
>  {
>  	const struct exception_table_entry *fixup;
> +	unsigned long insn_ip;
>  	unsigned long new_ip;
>  
>  	fixup = search_exception_tables(*ip);
>  	if (fixup) {
> +		insn_ip = ex_insn_addr(fixup);
>  		new_ip = ex_fixup_addr(fixup);
>  
> -		if (fixup->fixup - fixup->insn >= 0x7ffffff0 - 4) {
> +		/* See fixup_exception for details ... */
> +		if (new_ip - insn_ip >= 0x7ffffff0) {
>  			/* uaccess handling not supported during early boot */
>  			return 0;
>  		}
> 


-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: OpenPGP digital signature
URL: <https://lists.ubuntu.com/archives/kernel-team/attachments/20180223/8cda3124/attachment.sig>


More information about the kernel-team mailing list