No, I don't have any BPFs in test.
If we have Application Data in TCP queue then tls_sw_advance_skb
will change ctx->control from 0x16 to 0x17 (TLS_RECORD_TYPE_DATA)
and the loop will continue. The patched if will make zc = true and
data will be decrypted into msg->msg_iter.
After that the loop will break on:
if (!control)
control = tlm->control;
else if (control != tlm->control)
goto recv_end;
and the data will be lost.
Next call to recvmsg will find ctx->decrypted set to true and will
copy the unencrypted data from skb assuming that it has been decrypted
already.
The patch that I put into Fixes: changed the check you mentioned to
ctx->control, but originally it was checking the value of control that
was stored before calling to tls_sw_advance_skb.
On 15.11.2020 02:12, Jakub Kicinski wrote:
On Sat, 14 Nov 2020 07:09:42 +0300 Vadim Fedorenko wrote:
If tcp socket has more data than Encrypted Handshake Message then
tls_sw_recvmsg will try to decrypt next record instead of returning
full control message to userspace as mentioned in comment. The next
message - usually Application Data - gets corrupted because it uses
zero copy for decryption that's why the data is not stored in skb
for next iteration. Disable zero copy for this case.
Fixes: 692d7b5d1f91 ("tls: Fix recvmsg() to be able to peek across multiple
records")
Signed-off-by: Vadim Fedorenko <vfedore...@novek.ru>
Do you have some BPF in the mix?
I don't see how we would try to go past a non-data record otherwise,
since the loop ends like this:
if (tls_sw_advance_skb(sk, skb, chunk)) {
/* Return full control message to
* userspace before trying to parse
* another message type
*/
msg->msg_flags |= MSG_EOR;
if (ctx->control != TLS_RECORD_TYPE_DATA)
goto recv_end;
} else {
break;
}