diff mbox

[RFC,0/1] net/tls(TLS_SW): Data integrity issue with sw kTLS using sendfile

Message ID 1591109733-14159-1-git-send-email-pooja.trivedi@stackpath.com
State RFC
Delegated to: David Miller
Headers show

Commit Message

Pooja Trivedi June 2, 2020, 2:55 p.m. UTC
When sendfile is used for kTLS file delivery and 
the size provided to sendfile via its 'count'
parameter is greater than the file size, kTLS fails
to send the file correctly. The last chunk of the 
file is not sent, and the data integrity of the 
file is compromised on the receiver side. 
Based on studying the sendfile source code, in
such a case, last chunk of the file will be passed
with the MSG_MORE flag set. Following snippet from
fs/splice.c:1814 shows code within the while loop 
in splice_direct_to_actor() function that sets this
flag:

--------

	/*
	 * If more data is pending, set SPLICE_F_MORE
	 * If this is the last data and SPLICE_F_MORE 
	 * was not set initially, clears it.
	 */
	if (read_len < len)
		sd->flags |= SPLICE_F_MORE;
	else if (!more)
		sd->flags &= ~SPLICE_F_MORE;

--------

Due to this, tls layer adds the chunk to the pending 
records, but does not push it. Following lines of code
from tls_sw_do_sendpage() function in tls_sw.c:1153 show 
the end of record (eor) variable being set based on 
MSG_MORE flag:

--------

	bool eor;

	eor = !(flags & (MSG_MORE | MSG_SENDPAGE_NOTLAST));

--------

This eor bool is then used in the condition check for 
full_record, end of record, or sk_msg_full in 
tls_sw_do_sendpage() function in tls_sw.c:1212:

--------

	if (full_record || eor || sk_msg_full(msg_pl)) {
		ret = bpf_exec_tx_verdict(msg_pl, sk, full_record,
				  record_type, &copied, flags);
		if (ret) {
			if (ret == -EINPROGRESS)
				num_async++;
			else if (ret == -ENOMEM)
				goto wait_for_memory;
			else if (ret != -EAGAIN) {
				if (ret == -ENOSPC)
					ret = 0;
				goto sendpage_end;
			}
		}
	}
	continue;

--------

Changing the code in splice_direct_to_actor() function 
in fs/splice.c to detect end of file by checking 'pos'
variable against file size, and setting MSG_MORE flag
only when EOF is not reached, fixes the issue:

that helps reproduce the issue.
diff mbox

Patch

--- a/fs/splice.c
+++ b/fs/splice.c
@@ -980,10 +980,12 @@  ssize_t splice_direct_to_actor(struct file *in, struct splice_desc *sd,
                 * If this is the last data and SPLICE_F_MORE was not set
                 * initially, clears it.
                 */
-               if (read_len < len)
-                       sd->flags |= SPLICE_F_MORE;
-               else if (!more)
+               if (read_len < len) {
+                       if (pos < i_size_read(file_inode(in)))
+                               sd->flags |= SPLICE_F_MORE;
+               } else if (!more)
                        sd->flags &= ~SPLICE_F_MORE;
+               }


Sending a followup patch to this that adds a selftest