On Tue, Apr 04, 2017 at 03:46:51PM +0100, James Hogan wrote: > @@ -618,7 +617,7 @@ EXPORT_SYMBOL(__copy_user); > " GETB D1Ar1,[%1++]\n" \ > "2: SETB [%0++],D1Ar1\n", \ > "3: ADD %2,%2,#1\n" \ > - " SETB [%0++],D1Ar1\n", \ > + " ADD %0,%0,#1\n", \ Why bother incrementing dst here? > " .long 2b,3b\n") > > #define __asm_copy_from_user_2x_cont(to, from, ret, COPY, FIXUP, TENTRY) \ > @@ -626,7 +625,7 @@ EXPORT_SYMBOL(__copy_user); > " GETW D1Ar1,[%1++]\n" \ > "2: SETW [%0++],D1Ar1\n" COPY, \ > "3: ADD %2,%2,#2\n" \ > - " SETW [%0++],D1Ar1\n" FIXUP, \ > + " ADD %0,%0,#2\n" FIXUP, \ > " .long 2b,3b\n" TENTRY) > > #define __asm_copy_from_user_2(to, from, ret) \ > @@ -637,7 +636,7 @@ EXPORT_SYMBOL(__copy_user); > " GETB D1Ar1,[%1++]\n" \ > "4: SETB [%0++],D1Ar1\n", \ > "5: ADD %2,%2,#1\n" \ > - " SETB [%0++],D1Ar1\n", \ > + " ADD %0,%0,#1\n", \ > " .long 4b,5b\n") > > #define __asm_copy_from_user_4x_cont(to, from, ret, COPY, FIXUP, TENTRY) \ > @@ -645,23 +644,20 @@ EXPORT_SYMBOL(__copy_user); > " GETD D1Ar1,[%1++]\n" \ > "2: SETD [%0++],D1Ar1\n" COPY, \ > "3: ADD %2,%2,#4\n" \ > - " SETD [%0++],D1Ar1\n" FIXUP, \ > + " ADD %0,%0,#4\n" FIXUP, \ > " .long 2b,3b\n" TENTRY) > > #define __asm_copy_from_user_8x64(to, from, ret) \ > asm volatile ( \ > " GETL D0Ar2,D1Ar1,[%1++]\n" \ > "2: SETL [%0++],D0Ar2,D1Ar1\n" \ > "1:\n" \ > " .section .fixup,\"ax\"\n" \ > - " MOV D1Ar1,#0\n" \ > - " MOV D0Ar2,#0\n" \ > "3: ADD %2,%2,#8\n" \ > - " SETL [%0++],D0Ar2,D1Ar1\n" \ > + " ADD %0,%0,#8\n" \ > " MOVT D0Ar2,#HI(1b)\n" \ > " JUMP D0Ar2,#LO(1b)\n" \ > " .previous\n" \ Ditto for all of those, actually. > +/* > + * Copy from user to kernel. The return-value is the number of bytes that were > + * inaccessible. > + */ > +unsigned long raw_copy_from_user(void *pdst, const void __user *psrc, > + unsigned long n) > { > register char *dst asm ("A0.2") = pdst; > register const char __user *src asm ("A1.2") = psrc; > @@ -724,7 +721,7 @@ unsigned long __copy_user_zeroing(void *pdst, const void __user *psrc, > __asm_copy_from_user_1(dst, src, retn); > n--; > if (retn) > - goto copy_exception_bytes; > + return retn + n; > } > } Umm... What happens if psrc points to the last byte of an unmapped page, with psrc + 1 pointing to valid data? AFAICS, you'll get GETB fail, have retn increased to 1, n decremented by 1, then, assuming you end up in that byte-copy loop, have GETB at src + 1 *succeed*, the value fetched fed to SETB at dst + 1, n decremented again, non-zero retn finally spotted and n + retn returned, reporting that you've copied a single byte. Which you certainly have, but it's pdst[1] = psrc[1], not pdst[0] = psrc[0]. IOW, you need to check retn after all __asm_copy_from_user_{1,2} in the beginning. As for topic branch for #1 and #2 - sure, perfectly fine by me. Just give me the branch name and I'll pull. But I think the scenario above needs to be dealt with...