lkml.org 
[lkml]   [2021]   [Nov]   [1]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    From
    Subject[PATCH 5.10 19/77] net/tls: Fix flipped sign in tls_err_abort() calls
    Date
    From: Daniel Jordan <daniel.m.jordan@oracle.com>

    commit da353fac65fede6b8b4cfe207f0d9408e3121105 upstream.

    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>
    Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
    ---
    include/net/tls.h | 9 ++-------
    net/tls/tls_sw.c | 17 +++++++++++++----
    2 files changed, 15 insertions(+), 11 deletions(-)

    --- a/include/net/tls.h
    +++ b/include/net/tls.h
    @@ -359,6 +359,7 @@ int tls_sk_query(struct sock *sk, int op
    int __user *optlen);
    int tls_sk_attach(struct sock *sk, int optname, char __user *optval,
    unsigned int optlen);
    +void tls_err_abort(struct sock *sk, int err);

    int tls_set_sw_offload(struct sock *sk, struct tls_context *ctx, int tx);
    void tls_sw_strparser_arm(struct sock *sk, struct tls_context *ctx);
    @@ -467,12 +468,6 @@ static inline bool tls_is_sk_tx_device_o
    #endif
    }

    -static inline void tls_err_abort(struct sock *sk, int err)
    -{
    - sk->sk_err = err;
    - sk->sk_error_report(sk);
    -}
    -
    static inline bool tls_bigint_increment(unsigned char *seq, int len)
    {
    int i;
    @@ -513,7 +508,7 @@ static inline void tls_advance_record_sn
    struct cipher_context *ctx)
    {
    if (tls_bigint_increment(ctx->rec_seq, prot->rec_seq_size))
    - tls_err_abort(sk, EBADMSG);
    + tls_err_abort(sk, -EBADMSG);

    if (prot->version != TLS_1_3_VERSION)
    tls_bigint_increment(ctx->iv + TLS_CIPHER_AES_GCM_128_SALT_SIZE,
    --- a/net/tls/tls_sw.c
    +++ b/net/tls/tls_sw.c
    @@ -35,6 +35,7 @@
    * SOFTWARE.
    */

    +#include <linux/bug.h>
    #include <linux/sched/signal.h>
    #include <linux/module.h>
    #include <linux/splice.h>
    @@ -43,6 +44,14 @@
    #include <net/strparser.h>
    #include <net/tls.h>

    +noinline void tls_err_abort(struct sock *sk, int err)
    +{
    + WARN_ON_ONCE(err >= 0);
    + /* sk->sk_err should contain a positive error code. */
    + sk->sk_err = -err;
    + sk->sk_error_report(sk);
    +}
    +
    static int __skb_nsg(struct sk_buff *skb, int offset, int len,
    unsigned int recursion_level)
    {
    @@ -419,7 +428,7 @@ int tls_tx_records(struct sock *sk, int

    tx_err:
    if (rc < 0 && rc != -EAGAIN)
    - tls_err_abort(sk, EBADMSG);
    + tls_err_abort(sk, -EBADMSG);

    return rc;
    }
    @@ -764,7 +773,7 @@ static int tls_push_record(struct sock *
    msg_pl->sg.size + prot->tail_size, i);
    if (rc < 0) {
    if (rc != -EINPROGRESS) {
    - tls_err_abort(sk, EBADMSG);
    + tls_err_abort(sk, -EBADMSG);
    if (split) {
    tls_ctx->pending_open_record_frags = true;
    tls_merge_open_record(sk, rec, tmp, orig_end);
    @@ -1828,7 +1837,7 @@ int tls_sw_recvmsg(struct sock *sk,
    err = decrypt_skb_update(sk, skb, &msg->msg_iter,
    &chunk, &zc, async_capable);
    if (err < 0 && err != -EINPROGRESS) {
    - tls_err_abort(sk, EBADMSG);
    + tls_err_abort(sk, -EBADMSG);
    goto recv_end;
    }

    @@ -2008,7 +2017,7 @@ ssize_t tls_sw_splice_read(struct socket
    }

    if (err < 0) {
    - tls_err_abort(sk, EBADMSG);
    + tls_err_abort(sk, -EBADMSG);
    goto splice_read_end;
    }
    ctx->decrypted = 1;

    \
     
     \ /
      Last update: 2021-11-01 10:34    [W:4.389 / U:0.008 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site