Linux kernel mirror (for testing) git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
kernel os linux

net/tls: Fix flipped sign in tls_err_abort() calls

sk->sk_err appears to expect a positive value, a convention that ktls
doesn't always follow and that leads to memory corruption in other code.
For instance,

[kworker]
tls_encrypt_done(..., err=<negative error from crypto request>)
tls_err_abort(.., err)
sk->sk_err = err;

[task]
splice_from_pipe_feed
...
tls_sw_do_sendpage
if (sk->sk_err) {
ret = -sk->sk_err; // ret is positive

splice_from_pipe_feed (continued)
ret = actor(...) // ret is still positive and interpreted as bytes
// written, resulting in underflow of buf->len and
// sd->len, leading to huge buf->offset and bogus
// addresses computed in later calls to actor()

Fix all tls_err_abort() callers to pass a negative error code
consistently and centralize the error-prone sign flip there, throwing in
a warning to catch future misuse and uninlining the function so it
really does only warn once.

Cc: stable@vger.kernel.org
Fixes: c46234ebb4d1e ("tls: RX path for ktls")
Reported-by: syzbot+b187b77c8474f9648fae@syzkaller.appspotmail.com
Signed-off-by: Daniel Jordan <daniel.m.jordan@oracle.com>
Signed-off-by: David S. Miller <davem@davemloft.net>

authored by

Daniel Jordan and committed by
David S. Miller
da353fac a32f07d2

+15 -11
+2 -7
include/net/tls.h
··· 358 358 int __user *optlen); 359 359 int tls_sk_attach(struct sock *sk, int optname, char __user *optval, 360 360 unsigned int optlen); 361 + void tls_err_abort(struct sock *sk, int err); 361 362 362 363 int tls_set_sw_offload(struct sock *sk, struct tls_context *ctx, int tx); 363 364 void tls_sw_strparser_arm(struct sock *sk, struct tls_context *ctx); ··· 467 466 #endif 468 467 } 469 468 470 - static inline void tls_err_abort(struct sock *sk, int err) 471 - { 472 - sk->sk_err = err; 473 - sk_error_report(sk); 474 - } 475 - 476 469 static inline bool tls_bigint_increment(unsigned char *seq, int len) 477 470 { 478 471 int i; ··· 507 512 struct cipher_context *ctx) 508 513 { 509 514 if (tls_bigint_increment(ctx->rec_seq, prot->rec_seq_size)) 510 - tls_err_abort(sk, EBADMSG); 515 + tls_err_abort(sk, -EBADMSG); 511 516 512 517 if (prot->version != TLS_1_3_VERSION && 513 518 prot->cipher_type != TLS_CIPHER_CHACHA20_POLY1305)
+13 -4
net/tls/tls_sw.c
··· 35 35 * SOFTWARE. 36 36 */ 37 37 38 + #include <linux/bug.h> 38 39 #include <linux/sched/signal.h> 39 40 #include <linux/module.h> 40 41 #include <linux/splice.h> ··· 43 42 44 43 #include <net/strparser.h> 45 44 #include <net/tls.h> 45 + 46 + noinline void tls_err_abort(struct sock *sk, int err) 47 + { 48 + WARN_ON_ONCE(err >= 0); 49 + /* sk->sk_err should contain a positive error code. */ 50 + sk->sk_err = -err; 51 + sk_error_report(sk); 52 + } 46 53 47 54 static int __skb_nsg(struct sk_buff *skb, int offset, int len, 48 55 unsigned int recursion_level) ··· 428 419 429 420 tx_err: 430 421 if (rc < 0 && rc != -EAGAIN) 431 - tls_err_abort(sk, EBADMSG); 422 + tls_err_abort(sk, -EBADMSG); 432 423 433 424 return rc; 434 425 } ··· 772 763 msg_pl->sg.size + prot->tail_size, i); 773 764 if (rc < 0) { 774 765 if (rc != -EINPROGRESS) { 775 - tls_err_abort(sk, EBADMSG); 766 + tls_err_abort(sk, -EBADMSG); 776 767 if (split) { 777 768 tls_ctx->pending_open_record_frags = true; 778 769 tls_merge_open_record(sk, rec, tmp, orig_end); ··· 1836 1827 err = decrypt_skb_update(sk, skb, &msg->msg_iter, 1837 1828 &chunk, &zc, async_capable); 1838 1829 if (err < 0 && err != -EINPROGRESS) { 1839 - tls_err_abort(sk, EBADMSG); 1830 + tls_err_abort(sk, -EBADMSG); 1840 1831 goto recv_end; 1841 1832 } 1842 1833 ··· 2016 2007 } 2017 2008 2018 2009 if (err < 0) { 2019 - tls_err_abort(sk, EBADMSG); 2010 + tls_err_abort(sk, -EBADMSG); 2020 2011 goto splice_read_end; 2021 2012 } 2022 2013 ctx->decrypted = 1;