netdev - Re: KCM - recvmsg() mangles packets?

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Sun, 5 Aug 2018 08:44:10 +0200
From:   Dominique Martinet <asmadeus@...ewreck.org>
To:     Tom Herbert <tom@...ntonium.net>
Cc:     Linux Kernel Network Developers <netdev@...r.kernel.org>
Subject: Re: KCM - recvmsg() mangles packets?

Dominique Martinet wrote on Sat, Aug 04, 2018:
> I talked too fast, I can get this to fail on later packets e.g.
> Got 18, expected 31 on 452nd message: 453453453453453453; flags: 80
> 
> The content is 453 in a loop so this really is the 453rd packet...
> 
> But being slower e.g. doing that usleep after every single packets and I
> could let the loop run until 100k without a hintch.
> 
> 
> There really has to be something wrong, I just can't tell what from
> looking at the code with my naive eyes.

I've spent a few hours debugging this weekend, I'm still not confident I
understand how this all works but it's a bit better than Friday.

I've tried updating kcm_recvmsg()'s use of kcm_wait_data() to use
skb_dequeue() instead of skb_peek(), with the idea that if the skb is
off the list it would have less chance to be messed with, and that
didn't seem to change anything so I was probably looking at the wrong
place.

So I went one step up to how we split the packet and the interface with
strparser and added an extra check of the protocol in kcm_rcv_strparser.

If I understand correctly, with stm = strp_msg(skb), stm->full_len
contains the output of the bpf progran and must match something like
this in the network byte-order version:
ntohl(*((u32 *)(skb->data + stm->offset)))

(I'm not sure about offset, since we pass the full skb to parse message,
wouldn't it look at the start of the buffer everytime? Well, offset
seems to be 0 everytime the first time that check fails so I can
probably ignore that for now...)

Just like the test program, that extra check seems to work but will fail
everytime the test program detects and error, although the data I can
see does not always match what the program sees, the packet would seem
to already be incorrect when it is added to the kcm queue... So there
must be something happening to the skb between parse_msg() and rcv_msg()
in __strp_recv() ...?

>From what I can see it's looking at a cloned skb so the headers are
private but the data is shared, but it looks like the tcp socket (csock
in kcm_attach) is properly locked at this time, so I'm not sure what
could corrupt the buffer.

I'll keep looking at this a bit more but any help is appreciated,

Thanks,
-- 
Dominique Martinet | Asmadeus