On Sun, Apr 24, 2022 at 12:37 PM Borislav Petkov <bp@xxxxxxxxx> wrote:
>
> You could give me some more details but AFAIU, you mean, that
> fallback to byte-sized reads is unnecessary and I can get rid of
> copy_user_handle_tail? Because that would be a nice cleanup.
Yeah, we already don't have that fallback in many other places.
For example: traditionally we implemented fixed-size small
copy_to/from_user() with get/put_user().
We don't do that any more, but see commit 4b842e4e25b1 ("x86: get rid
of small constant size cases in raw_copy_{to,from}_user()") and
realize how it historically never did the byte-loop fallback.
The "clear_user()" case is another example of something that was never
byte-exact.
And last time we discussed this, Al was looking at making it
byte-exact, and I'm pretty sure he noted that other architectures
already didn't do always do it.
Let me go try to find it.
> Anyway, I ran your short prog and it all looks like you predicted it:
Well, this actually shows a bug.
> fsrm:
> ----
> read(3, "\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0", 65536) = 17
The above is good.
> erms:
> -----
> read(3, "\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0", 65536) = 17
This is good too: erms should do small copies with the expanded case,
but sicne it *thinks* it's a large copy, it will just use "rep movsb"
and be byte-exact.
> rep_good:
> ---------
> read(3, "\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0", 65536) = 16
This is good: it starts off with "rep movsq", does two iterations, and
then fails on the third word, so it only succeeds for 16 bytes.
> original:
> ---------
> read(3, strace: umoven: short read (17 < 33) @0x7f3ff61d5fef
> 0x7f3ff61d5fef, 65536) = 3586
This is broken.
And I'm *not* talking about the strace warning. The strace warning is
actually just a *result* of the kernel bug.
Look at that return value. It returns '3586'. That's just pure garbage.
So that 'original' routine simply returns the wrong value.
I suspect it's a %rax vs %rcx confusion again, but with your "patch on
top of earlier patch" I didn't go and sort it out.
Linus
