Messages in this thread Patch in this message | | | From | Thomas Gleixner <> | Subject | Re: AW: [PATCH] amd64: Fix csum_partial_copy_generic() | Date | Thu, 19 Oct 2023 13:45:48 +0200 |
| |
On Thu, Oct 19 2023 at 07:39, Al Viro wrote: > On Thu, Oct 19, 2023 at 07:14:27AM +0100, Al Viro wrote: >> > > Here's our situation: >> > > Our device gets pinged by a third party manufacturer robot controller. >> > > We have updated the kernel in our device to 5.15 from 4.9, the robot >> > > controller is kept unchanged. At 4.9, our device's ping reply is accepted >> > > by the robot controller, at 5.15 it's not. >> > > >> > > Wireshark shows a bad checksum warning: >> > > 'Checksum: 0x0000 incorrect, should be 0xffff' >> > > >> > >> > Lovely. I think I see what's going on, give me a few to think about it... >> >> The real source of trouble was switching csum_and_copy_{to,from}_user() >> to reporting faults as 0. And yes, it's broken. Bugger... > > I really hate the idea of bringing back the old horrors and splitting > _nocheck and _user variants ;-/ Especially since we don't care (and > never had, really) where in the EFAULT case had the damn thing faulted > and what csum had it managed to accumulate prior to that point. > > The only callers are csum_and_copy_..._iter() and they discard > the entire iovec segment if fault happens; all users of > csum_and_copy_from_iter() are actually discarding everything in > that case (reverting iterator to the point where it had been > prior to the call). > > And they are all thread-synchronous. Hell, it's tempting to steal > a thread flag, clear it before the call of those suckers, set it in > exception handlers in those and check in csum_and_copy_..._iter() > after the call... Let me see how ugly something of that sort would > be...
Eew. That's horrible.
The checksum is strictly 16bit. __wsum is 32bit (for whatever reason). So you can differentiate between error and valid checksum easily by using bit 16-31 as indicator for fail or success, no?
Something like the incomplete below.
Thanks,
tglx
--- --- a/arch/x86/lib/csum-copy_64.S +++ b/arch/x86/lib/csum-copy_64.S @@ -194,6 +194,9 @@ SYM_FUNC_START(csum_partial_copy_generic .Lende: testq %r10, %r10 js .Lwas_odd + +.Lsuccess: + orl $0xFF00, %eax .Lout: movq 0*8(%rsp), %rbx movq 1*8(%rsp), %r12 @@ -247,7 +250,7 @@ SYM_FUNC_START(csum_partial_copy_generic .Lwas_odd: roll $8, %eax - jmp .Lout + jmp .Lsuccess /* Exception: just return 0 */ .Lfault:
| |