[PATCH 2/3] arm64: Update copy_to_user()

Will Deacon will at kernel.org
Mon Dec 5 08:09:10 PST 2022


Hi Robin,

On Wed, Sep 28, 2022 at 12:58:52PM +0100, Robin Murphy wrote:
> As with its counterpart, replace copy_to_user() with a new and improved
> version similarly derived from memcpy(). Different tradeoffs from the
> base implementation are made relative to copy_from_user() to get the
> most consistent results across different microarchitectures, but the
> overall shape of the performance gain ends up about the same.
> 
> The exception fixups are even more comical this time around, but that's
> down to now needing to reconstruct the faulting address, and cope with
> overlapping stores at various points. Again, improvements to the
> exception mechanism could significantly simplify things in future.
> 
> Signed-off-by: Robin Murphy <robin.murphy at arm.com>
> ---
>  arch/arm64/lib/copy_to_user.S | 391 +++++++++++++++++++++++++++++-----
>  1 file changed, 341 insertions(+), 50 deletions(-)

Mark reported (off-list) a KASAN splat from Syzkaller on for-next/core:

  | BUG: KASAN: stack-out-of-bounds in _copy_to_iter+0x1084/0x131c

We've not had any reports of this before (or on other branches) and Mark
is unable to reproduce the problem on mainline. I suggested trying
for-next/uaccess and he confirmed that the problem has been introduced
there. Hopefully he can chime in with more details.

In the meantime, I think it's safest to drop this branch for now considering
where we are in the release cycle. I'll leave it pushed to the arm64 git,
so we can put fixes on top and try to merge it next time around instead.

One thing I _did_ notice from a quick skim of the code is:

> +	/* Fixups... */
> +
> +	/*
> +	 * Fault on the first write, but progress may have been possible;
> +	 * realign dst and retry a single byte to confirm.
> +	 */
> +L(fixup_pre):
> +	mov	dst, dstin
> +U_dst(	ldtrb	A_lw, [src])
> +	strb	A_lw, [dst], #1
> +L(fixup_dst):
> +	sub	x0, dstend, dst
> +	ret

Why is U_dst() being applied to the load for a copy_to_user() here? I
would've expected to annotate the store.

Will



More information about the linux-arm-kernel mailing list