tls: Fix socket mem accounting error under async encryption
authorVakul Garg <vakul.garg@nxp.com>
Tue, 25 Sep 2018 10:56:17 +0000 (16:26 +0530)
committerDavid S. Miller <davem@davemloft.net>
Tue, 25 Sep 2018 17:43:01 +0000 (10:43 -0700)
Current async encryption implementation sometimes showed up socket
memory accounting error during socket close. This results in kernel
warning calltrace. The root cause of the problem is that socket var
sk_forward_alloc gets corrupted due to access in sk_mem_charge()
and sk_mem_uncharge() being invoked from multiple concurrent contexts
in multicore processor. The apis sk_mem_charge() and sk_mem_uncharge()
are called from functions alloc_plaintext_sg(), free_sg() etc. It is
required that memory accounting apis are called under a socket lock.

The plaintext sg data sent for encryption is freed using free_sg() in
tls_encryption_done(). It is wrong to call free_sg() from this function.
This is because this function may run in irq context. We cannot acquire
socket lock in this function.

We remove calling of function free_sg() for plaintext data from
tls_encryption_done() and defer freeing up of plaintext data to the time
when the record is picked up from tx_list and transmitted/freed. When
tls_tx_records() gets called, socket is already locked and thus there is
no concurrent access problem.

Fixes: a42055e8d2c3 ("net/tls: Add support for async encryption")
Signed-off-by: Vakul Garg <vakul.garg@nxp.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
net/tls/tls_sw.c

index bf03f32..406d3bb 100644 (file)
@@ -353,6 +353,9 @@ int tls_tx_records(struct sock *sk, int flags)
                 * Remove the head of tx_list
                 */
                list_del(&rec->list);
+               free_sg(sk, rec->sg_plaintext_data,
+                       &rec->sg_plaintext_num_elem, &rec->sg_plaintext_size);
+
                kfree(rec);
        }
 
@@ -371,6 +374,10 @@ int tls_tx_records(struct sock *sk, int flags)
                                goto tx_err;
 
                        list_del(&rec->list);
+                       free_sg(sk, rec->sg_plaintext_data,
+                               &rec->sg_plaintext_num_elem,
+                               &rec->sg_plaintext_size);
+
                        kfree(rec);
                } else {
                        break;
@@ -399,8 +406,6 @@ static void tls_encrypt_done(struct crypto_async_request *req, int err)
        rec->sg_encrypted_data[0].offset -= tls_ctx->tx.prepend_size;
        rec->sg_encrypted_data[0].length += tls_ctx->tx.prepend_size;
 
-       free_sg(sk, rec->sg_plaintext_data,
-               &rec->sg_plaintext_num_elem, &rec->sg_plaintext_size);
 
        /* Free the record if error is previously set on socket */
        if (err || sk->sk_err) {
@@ -523,9 +528,6 @@ static int tls_push_record(struct sock *sk, int flags,
        if (rc == -EINPROGRESS)
                return -EINPROGRESS;
 
-       free_sg(sk, rec->sg_plaintext_data, &rec->sg_plaintext_num_elem,
-               &rec->sg_plaintext_size);
-
        if (rc < 0) {
                tls_err_abort(sk, EBADMSG);
                return rc;
@@ -1566,6 +1568,11 @@ void tls_sw_free_resources_tx(struct sock *sk)
 
                rec = list_first_entry(&ctx->tx_list,
                                       struct tls_rec, list);
+
+               free_sg(sk, rec->sg_plaintext_data,
+                       &rec->sg_plaintext_num_elem,
+                       &rec->sg_plaintext_size);
+
                list_del(&rec->list);
                kfree(rec);
        }
@@ -1575,6 +1582,10 @@ void tls_sw_free_resources_tx(struct sock *sk)
                        &rec->sg_encrypted_num_elem,
                        &rec->sg_encrypted_size);
 
+               free_sg(sk, rec->sg_plaintext_data,
+                       &rec->sg_plaintext_num_elem,
+                       &rec->sg_plaintext_size);
+
                list_del(&rec->list);
                kfree(rec);
        }