Hi Linus,
On 9/07/21 6:20 am, Linus Torvalds wrote:
On Thu, Jul 8, 2021 at 5:57 AM Christoph Hellwig <hch@xxxxxx> wrote:
I've force pushed a new version to the branch, can you give it a spin?
Please stop playing broken games with __constant_copy_to_user().
Now you didn't just break the return value, you broke the actual copy
too. When it is supposed to do a 4-byte copy, the code now does *two*
4-byte copies, because that's the way __constant_copy_to_user_asm()
works - it always does at least two accesses, and then the third one
is conditional.
So that "6, l, l, )" in
case 4:
__constant_copy_to_user_asm(res, to, from, tmp, 6, l, l,);
break;
literally means "try to do 2x 'l' sized moves (but not a third one),
and return 6 if it fails". All of which is very wrong indeed.
In order to get the correct number of bytes copied, the patch would have
to look like:
switch (n) {
case 1:
- __put_user_asm(res, *(u8 *)from, (u8 __user *)to, b, d, 1);
+ __constant_copy_to_user_asm(res, to, from, tmp, 1, b, );
break;
case 2:
- __put_user_asm(res, *(u16 *)from, (u16 __user *)to, w,
r, 2);
+ __constant_copy_to_user_asm(res, to, from, tmp, 2, w, );
break;
case 3:
__constant_copy_to_user_asm(res, to, from, tmp, 3, w, b,);
break;
case 4:
- __put_user_asm(res, *(u32 *)from, (u32 __user *)to, l,
r, 4);
+ __constant_copy_to_user_asm(res, to, from, tmp, 4, l, );
break;
and __constant_copy_to_user_asm() changed to deal with an empty s2
parameter. Probably too much work.
So commit d36105c942e0 ("m68k: simplify the __constant_copy_to_user
implementation") is very very broken.
Doesn't appear to matter - though I'll back out those changes to be safe.
Note that in this version, faults in __put_user_asm used from
__constant_copy_to_user will return -EFAULT, not the remaining number of
bytes to be copied, which might get confusing for the caller.
Cheers,
Michael
But the rest looks good to me. Of course, I entirely missed the fact
that Andreas pointed out - "instr" was inside a string - so who knows
what I missed this time.
Linus
