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 <[email protected]>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; }
