* [PATCH net v2] net/tls: sendfile fails with ktls offload
@ 2020-10-05 18:43 Rohit Maheshwari
2020-10-05 23:44 ` Jakub Kicinski
0 siblings, 1 reply; 2+ messages in thread
From: Rohit Maheshwari @ 2020-10-05 18:43 UTC (permalink / raw)
To: kuba, netdev, davem; +Cc: secdev, Rohit Maheshwari
At first when sendpage gets called, if there is more data, 'more' in
tls_push_data() gets set which later sets pending_open_record_frags, but
when there is no more data in file left, and last time tls_push_data()
gets called, pending_open_record_frags doesn't get reset. And later when
2 bytes of encrypted alert comes as sendmsg, it first checks for
pending_open_record_frags, and since this is set, it creates a record with
0 data bytes to encrypt, meaning record length is prepend_size + tag_size
only, which causes problem.
We should set/reset pending_open_record_frags based on more bit.
Also incase if tls_do_allocation() fails, and if record len is only
prepend_size, then destroy the record.
v1->v2:
- handle tls_do_allocation() failure handling.
Signed-off-by: Rohit Maheshwari <rohitm@chelsio.com>
---
net/tls/tls_device.c | 27 +++++++++++++--------------
1 file changed, 13 insertions(+), 14 deletions(-)
diff --git a/net/tls/tls_device.c b/net/tls/tls_device.c
index b74e2741f74f..f3efd53e31cf 100644
--- a/net/tls/tls_device.c
+++ b/net/tls/tls_device.c
@@ -463,17 +463,16 @@ static int tls_push_data(struct sock *sk,
if (!record)
break;
handle_error:
- if (record_type != TLS_RECORD_TYPE_DATA) {
- /* avoid sending partial
- * record with type !=
- * application_data
- */
- size = orig_size;
- destroy_record(record);
- ctx->open_record = NULL;
- } else if (record->len > prot->prepend_size) {
+ /* avoid sending partial record with type !=
+ * application_data
+ */
+ if (record_type == TLS_RECORD_TYPE_DATA &&
+ record->len > prot->prepend_size)
goto last_record;
- }
+
+ size = orig_size;
+ destroy_record(record);
+ ctx->open_record = NULL;
break;
}
@@ -492,11 +491,11 @@ static int tls_push_data(struct sock *sk,
if (!size) {
last_record:
tls_push_record_flags = flags;
- if (more) {
- tls_ctx->pending_open_record_frags =
- !!record->num_frags;
+ /* set/clear pending_open_record_frags based on more */
+ tls_ctx->pending_open_record_frags = !!more;
+
+ if (more)
break;
- }
done = true;
}
--
2.18.1
^ permalink raw reply related [flat|nested] 2+ messages in thread
* Re: [PATCH net v2] net/tls: sendfile fails with ktls offload
2020-10-05 18:43 [PATCH net v2] net/tls: sendfile fails with ktls offload Rohit Maheshwari
@ 2020-10-05 23:44 ` Jakub Kicinski
0 siblings, 0 replies; 2+ messages in thread
From: Jakub Kicinski @ 2020-10-05 23:44 UTC (permalink / raw)
To: Rohit Maheshwari; +Cc: netdev, davem, secdev
On Tue, 6 Oct 2020 00:13:13 +0530 Rohit Maheshwari wrote:
> At first when sendpage gets called, if there is more data, 'more' in
> tls_push_data() gets set which later sets pending_open_record_frags, but
> when there is no more data in file left, and last time tls_push_data()
> gets called, pending_open_record_frags doesn't get reset. And later when
> 2 bytes of encrypted alert comes as sendmsg, it first checks for
> pending_open_record_frags, and since this is set, it creates a record with
> 0 data bytes to encrypt, meaning record length is prepend_size + tag_size
> only, which causes problem.
> We should set/reset pending_open_record_frags based on more bit.
>
> Also incase if tls_do_allocation() fails, and if record len is only
> prepend_size, then destroy the record.
>
> v1->v2:
> - handle tls_do_allocation() failure handling.
>
> Signed-off-by: Rohit Maheshwari <rohitm@chelsio.com>
> ---
> net/tls/tls_device.c | 27 +++++++++++++--------------
> 1 file changed, 13 insertions(+), 14 deletions(-)
>
> diff --git a/net/tls/tls_device.c b/net/tls/tls_device.c
> index b74e2741f74f..f3efd53e31cf 100644
> --- a/net/tls/tls_device.c
> +++ b/net/tls/tls_device.c
> @@ -463,17 +463,16 @@ static int tls_push_data(struct sock *sk,
> if (!record)
> break;
> handle_error:
> - if (record_type != TLS_RECORD_TYPE_DATA) {
> - /* avoid sending partial
> - * record with type !=
> - * application_data
> - */
> - size = orig_size;
> - destroy_record(record);
> - ctx->open_record = NULL;
> - } else if (record->len > prot->prepend_size) {
> + /* avoid sending partial record with type !=
> + * application_data
> + */
> + if (record_type == TLS_RECORD_TYPE_DATA &&
> + record->len > prot->prepend_size)
> goto last_record;
> - }
> +
> + size = orig_size;
> + destroy_record(record);
> + ctx->open_record = NULL;
Yet, this still does not update pending_open_record_frags...
> break;
> }
> @@ -492,11 +491,11 @@ static int tls_push_data(struct sock *sk,
> if (!size) {
> last_record:
> tls_push_record_flags = flags;
> - if (more) {
> - tls_ctx->pending_open_record_frags =
> - !!record->num_frags;
> + /* set/clear pending_open_record_frags based on more */
> + tls_ctx->pending_open_record_frags = !!more;
> +
> + if (more)
> break;
> - }
>
> done = true;
> }
Maybe I'm misunderstanding what you're fixing but I think you should
just do this:
diff --git a/net/tls/tls_device.c b/net/tls/tls_device.c
index b74e2741f74f..674964d5684b 100644
--- a/net/tls/tls_device.c
+++ b/net/tls/tls_device.c
@@ -418,7 +418,6 @@ static int tls_push_data(struct sock *sk,
struct tls_context *tls_ctx = tls_get_ctx(sk);
struct tls_prot_info *prot = &tls_ctx->prot_info;
struct tls_offload_context_tx *ctx = tls_offload_ctx_tx(tls_ctx);
- int more = flags & (MSG_SENDPAGE_NOTLAST | MSG_MORE);
struct tls_record_info *record = ctx->open_record;
int tls_push_record_flags;
struct page_frag *pfrag;
@@ -426,6 +425,7 @@ static int tls_push_data(struct sock *sk,
u32 max_open_record_len;
int copy, rc = 0;
bool done = false;
+ bool more = false;
long timeo;
if (flags &
@@ -492,9 +492,8 @@ static int tls_push_data(struct sock *sk,
if (!size) {
last_record:
tls_push_record_flags = flags;
- if (more) {
- tls_ctx->pending_open_record_frags =
- !!record->num_frags;
+ if (flags & (MSG_SENDPAGE_NOTLAST | MSG_MORE)) {
+ more = true;
break;
}
@@ -526,6 +525,8 @@ static int tls_push_data(struct sock *sk,
}
} while (!done);
+ tls_ctx->pending_open_record_frags = more;
+
if (orig_size - size > 0)
rc = orig_size - size;
^ permalink raw reply related [flat|nested] 2+ messages in thread
end of thread, other threads:[~2020-10-05 23:45 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-10-05 18:43 [PATCH net v2] net/tls: sendfile fails with ktls offload Rohit Maheshwari
2020-10-05 23:44 ` Jakub Kicinski
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.