linux-kernel - Re: [PATCH 07/11] ARM: Add KGDB/KDB FIQ debugger generic code

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20120730140724.GI6802@n2100.arm.linux.org.uk>
Date:	Mon, 30 Jul 2012 15:07:24 +0100
From:	Russell King - ARM Linux <linux@....linux.org.uk>
To:	Anton Vorontsov <anton.vorontsov@...aro.org>
Cc:	Jason Wessel <jason.wessel@...driver.com>,
	Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
	Alan Cox <alan@...ux.intel.com>,
	Arve Hjønnevåg <arve@...roid.com>,
	Colin Cross <ccross@...roid.com>,
	John Stultz <john.stultz@...aro.org>,
	linux-kernel@...r.kernel.org, linux-arm-kernel@...ts.infradead.org,
	linaro-kernel@...ts.linaro.org, patches@...aro.org,
	kernel-team@...roid.com, kgdb-bugreport@...ts.sourceforge.net
Subject: Re: [PATCH 07/11] ARM: Add KGDB/KDB FIQ debugger generic code

On Mon, Jul 30, 2012 at 04:58:16AM -0700, Anton Vorontsov wrote:
> +	.align	5
> +__fiq_svc:
> +	svc_entry
> +	fiq_handler
> +	svc_exit r5				@ return from exception
> + UNWIND(.fnend		)
> +ENDPROC(__fiq_svc)
> +	.ltorg
> +
> +	.align	5
> +__fiq_usr:
> +	usr_entry
> +	kuser_cmpxchg_check
> +	fiq_handler
> +	get_thread_info tsk
> +	mov	why, #0
> +	b	ret_to_user_from_irq
> + UNWIND(.fnend		)
> +ENDPROC(__fiq_usr)
> +	.ltorg
> +
> +	.global kgdb_fiq_handler
> +kgdb_fiq_handler:
> +
> +	vector_stub	fiq, FIQ_MODE, 4
> +
> +	.long	__fiq_usr			@  0  (USR_26 / USR_32)
> +	.long	__fiq_svc			@  1  (FIQ_26 / FIQ_32)
> +	.long	__fiq_svc			@  2  (IRQ_26 / IRQ_32)
> +	.long	__fiq_svc			@  3  (SVC_26 / SVC_32)
> +	.long	__fiq_svc			@  4
> +	.long	__fiq_svc			@  5
> +	.long	__fiq_svc			@  6
> +	.long	__fiq_svc			@  7
> +	.long	__fiq_svc			@  8
> +	.long	__fiq_svc			@  9
> +	.long	__fiq_svc			@  a
> +	.long	__fiq_svc			@  b
> +	.long	__fiq_svc			@  c
> +	.long	__fiq_svc			@  d
> +	.long	__fiq_svc			@  e
> +	.long	__fiq_svc			@  f

I am not convinced that this does not cause loss of state from the parent
context.  Let's review what happens when a FIQ is received from SVC mode
with the above code.

- The CPU will be in SVC mode.
- FIQ received.
- CPU saves CPSR into SPSR_fiq and PC into LR_fiq, and jumps to the FIQ
  vector.
- We apply the 4 byte correction to LR_fiq, and store r0, LR_fiq and
  SPSR_fiq to the FIQ 'stack'
- We switch to SVC mode and jump to __fiq_svc
- svc_entry:
  - adjusts the SVC stack pointer down, and saves r1 - r12
  - loads r0, LR_fiq and SPSR_fiq and saves them as ARM_r0, ARM_pc, ARM_cpsr
    into the pt_regs
  - the original value of the SVC stack pointer is saved as ARM_r13
  - LR_svc is saved as ARM_r14

At this point, we have saved everything *except* for the SPSR_svc register.

Now, when we return from the above, we use svc_exit:
- write SPSR_svc with ARM_cpsr (from SPSR_fiq)
- load r0-pc from the pt_regs and load CPSR from SPSR_svc

Now the thing here is that even if we did preserve SPSR_svc, with the
above exit sequence, there is _no_ way to preserve the value of SPSR_svc.
Normally, this doesn't matter because we know that the regions we care
about this have IRQs disabled.

However, what this means, if we receive an FIQ and use this path from any
part of the kernel which expects SPSR_svc to be preserved (eg, the exit
path from any exception) the kernel will blow up.

I guess you could do something like this instead:
- disable FIQs
- load SPSR_svc with a saved value of it from entry.
- load r1-r14 from ARM_r1..ARM_lr
- switch to FIQ mode
- load SPSR_fiq from saved ARM_cpsr
- load r0 from ARM_r0
- load pc from ARM_pc

So, maybe something like this for the svc return path:

	cpsid	f
	ldr	r1, [saved_spsr_svc]
	mov	r0, sp
	mrs	spsr_cxsf, r1
	ldmib	r0, {r1 - r14}
	msr	cpsr_c, #FIQ_MODE | PSR_I_BIT | PSR_F_BIT
	add	r7, r0, #S_PC
	ldr	r8, [r0, #S_CPSR]
	mrs	spsr_cxsf, r8
	ldr	r0, [r0, #S_R0]
	ldmia	r7, {pc}^
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/