arm64: fix strnlen_user when count <= strlen
PATCH 1/1 arm64: fix strnlen_user when count <= strlen
I received a bug report about the ruby test-suite failing on AArch64 when attempting to pass MAX_ARG_STRLEN sized args to execv[1]. It was expecting an E2BIG returned, but instead was receiving ENOMEM, and concatenating the argument strings in funky ways.
The problem appeared to be in __strnlen_user on arm64, as when instrumenting fs/exec.c to compare the results of the asm-generic strnlen_user, I noticed an off-by-one on the result:
long-param-test: optimized strnlen_user (131072) and naive (131073) disagree!
As a result, fix strnlen_user to match expected behaviour as documented in lib/strnlen_user.c, and return count+1 when count would be exceeded.
I didn't feel comfortable prodding the assembler, so I just worked around it in the wrapper.
1. https://bugzilla.redhat.com/show_bug.cgi?id=1038676
---
I tested that this behaves the same as the lib/strnlen_user.c version with some hacked together code here:
http://kyle.fedorapeople.org/strnlen_user-test.tar.xz
{master}kmcmarti ~/strnlen_user-test $ ./test
=== count = 0 ===strnlen_user = 0__strnlen_user = 0fixed__strnlen_user = 0
=== count < strlen ===strnlen_user = 7__strnlen_user = 6fixed__strnlen_user = 7
=== count = strlen ===strnlen_user = 13__strnlen_user = 12fixed__strnlen_user = 13
=== count > strlen ===strnlen_user = 13__strnlen_user = 13fixed__strnlen_user = 13
arch/arm64/include/asm/uaccess.h +6 -0
@@ -290,9 +290,15 @@ static inline long __must_check strnlen_user(const char __user *s, long n) {unsigned long res =0 ; +if (unlikely (n <=0 )) +return 0 ; +if (__addr_ok (s)) res =__strnlen_user (s, n); +if (unlikely (res >= n)) +return n +1 ; +return res; }
Hi Kyle,
Quoted 10 lines · Kyle McMartin
I received a bug report about the ruby test-suite failing on AArch64 when attempting to pass
MAX_ARG_STRLENsized args to execv[1]. It was expecting an E2BIG returned, but instead was receiving ENOMEM, and concatenating the argument strings in funky ways.The problem appeared to be in
__strnlen_useron arm64, as when instrumentingfs/exec.cto compare the results of the asm-genericstrnlen_user, I noticed an off-by-one on the result:
long-param-test: optimizedstrnlen_user(131072) and naive (131073) disagree!
Quoted 5 lines
As a result, fix
strnlen_userto match expected behaviour as documented inlib/strnlen_user.c, and return count+1 when count would be exceeded.I didn't feel comfortable prodding the assembler, so I just worked around it in the wrapper.
Actually, I have removed strnlen_user for 3.14. Could you try your test case with our for-next branch please?
git://git.kernel.org/pub/scm/linux/kernel/git/arm64/linux.git for-next/core
As for the issue you spotted, we probably need a fix for that to go into stable kernels. Does the following (smaller patch) work for you?
Will
--->8
arch/arm64/lib/strnlen_user.S +1 -0
@@ -37,6 +37,7 @@ ENTRY(__strnlen_user) USER(9f, ldrb w3, [x0], #1 ) cbnz w3, 1b 2: sub x0, x0, x2 + cinc x0, x0, mi ret ENDPROC(__strnlen_user)
Quoted 5 lines · Will Deacon
Actually, I have removed
strnlen_userfor 3.14. Could you try your test case with our for-next branch please?
git://git.kernel.org/pub/scm/linux/kernel/git/arm64/linux.gitfor-next/core
This will work fine, I believe (I can't easily test on HW since we're dependent on $vendor drivers that aren't upstream yet) but in my testing yesterday I parroted the x86 word-at-a-time.h and lib/strnlen_user.c and that seemed to work.
Quoted 3 lines
As for the issue you spotted, we probably need a fix for that to go into stable kernels. Does the following (smaller patch) work for you?
This gets the count == 0 case wrong still, I believe.
=== count = 0 ===strnlen_user = 0__strnlen_user = 1
(Where strnlen_user is the lib/strnlen_user.c implementation.)
Testing if (n <= 0) return 0; before __strnlen_user fixes it though.
Given this is called in, I think, only two places in the kernel, maybe we should just backport the switch to generic for stable? I can't imagine it conflicting with anything.
regards, Kyle
Quoted 16 lines
Will
--->8
arch/arm64/lib/strnlen_user.S +1
-0@@ -37,6 +37,7 @@ ENTRY(__strnlen_user) USER(9f, ldrb w3, [x0], #1 ) cbnz w3, 1b 2: sub x0, x0, x2 + cinc x0, x0, mi ret ENDPROC(__strnlen_user)