On Wed, May 12, 2021 at 12:31:39PM +0100, Robin Murphy wrote:

On 2021-05-12 11:48, Mark Rutland wrote:

quoted

On Tue, May 11, 2021 at 05:12:38PM +0100, Robin Murphy wrote:

quoted

Rewrite __arch_clear_user() with regular
USER() annotations so that it's clearer what's going on, and take the
opportunity to minimise the branchiness in the most common paths, which
also allows the exception fixup to return a more accurate result.

IIUC this isn't always accurate for the {4,2,1}-byte cases; example
below. I'm not sure whether that's intentional since the commit message
says "more accurate" rather than "accurate".

Indeed, the "more" was definitely significant :)

:)

quoted

If we think that under-estimating is fine, I reckon it'd be worth a
comment to make that clear.

Indeed for smaller amounts there's no change in fixup behaviour at all, but
I have to assume that underestimating by up to 100% is probably OK since
we've been underestimating by fully 100% for nearly 10 years now. I don't
believe it's worth having any more complexity than necessary for the fault
case - grepping for clear_user() usage suggests that nobody really cares
about the return value beyond whether it's zero or not, so the minor
"improvement" here is more of a nice-to-have TBH.

The existing comment doesn't actually explain anything either, which is why
I didn't replace it, but I'm happy to add something if you like.

I don't have strong feelings either way, but I think that we should at
least document this, since that'll at least save us rehashing the same
point in future. :)

That said, IIUC to make this always accurate we only need two ADDs (diff
below). Since those will only be executed at most once each, I suspect
they won't have a measureable impact in practice.

So maybe it's worth adding them to avoid any risk that someone needs
this to be accurate in future?

Mark.

---->8----

diff --git a/arch/arm64/lib/clear_user.S b/arch/arm64/lib/clear_user.S
index 1005345b4066..7ef553ec2677 100644
--- a/arch/arm64/lib/clear_user.S
+++ b/arch/arm64/lib/clear_user.S

@@ -32,12 +32,14 @@ USER(9f, sttr       xzr, [x2, #-8])
 
 2:     tbz     x1, #2, 3f
 USER(9f, sttr  wzr, [x0])
+       add     x0, x0, #4
 USER(9f, sttr  wzr, [x2, #-4])
        mov     x0, #0
        ret
 
 3:     tbz     x1, #1, 4f
 USER(9f, sttrh wzr, [x0])
+       add     x0, x0, #2
 4:     tbz     x1, #0, 5f
 USER(9f, sttrb wzr, [x2, #-1])
 5:     mov     x0, #0

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel