* [PATCH net 1/7] chtls: Fix hardware tid leak
2021-01-06 4:29 [PATCH net 0/7] Bug fixes for chtls driver Ayush Sawal
@ 2021-01-06 4:29 ` Ayush Sawal
2021-01-06 4:29 ` [PATCH net 2/7] chtls: Remove invalid set_tcb call Ayush Sawal
` (6 subsequent siblings)
7 siblings, 0 replies; 9+ messages in thread
From: Ayush Sawal @ 2021-01-06 4:29 UTC (permalink / raw)
To: kuba, netdev, davem; +Cc: secdev, Ayush Sawal, Rohit Maheshwari
send_abort_rpl() is not calculating cpl_abort_req_rss offset and
ends up sending wrong TID with abort_rpl WR causng tid leaks.
Replaced send_abort_rpl() with chtls_send_abort_rpl() as it is
redundant.
Fixes: cc35c88ae4db ("crypto : chtls - CPL handler definition")
Signed-off-by: Rohit Maheshwari <rohitm@chelsio.com>
Signed-off-by: Ayush Sawal <ayush.sawal@chelsio.com>
---
.../chelsio/inline_crypto/chtls/chtls_cm.c | 39 ++-----------------
1 file changed, 3 insertions(+), 36 deletions(-)
diff --git a/drivers/net/ethernet/chelsio/inline_crypto/chtls/chtls_cm.c b/drivers/net/ethernet/chelsio/inline_crypto/chtls/chtls_cm.c
index a0e0d8a83681..561b5f2273af 100644
--- a/drivers/net/ethernet/chelsio/inline_crypto/chtls/chtls_cm.c
+++ b/drivers/net/ethernet/chelsio/inline_crypto/chtls/chtls_cm.c
@@ -1997,39 +1997,6 @@ static void t4_defer_reply(struct sk_buff *skb, struct chtls_dev *cdev,
spin_unlock_bh(&cdev->deferq.lock);
}
-static void send_abort_rpl(struct sock *sk, struct sk_buff *skb,
- struct chtls_dev *cdev, int status, int queue)
-{
- struct cpl_abort_req_rss *req = cplhdr(skb);
- struct sk_buff *reply_skb;
- struct chtls_sock *csk;
-
- csk = rcu_dereference_sk_user_data(sk);
-
- reply_skb = alloc_skb(sizeof(struct cpl_abort_rpl),
- GFP_KERNEL);
-
- if (!reply_skb) {
- req->status = (queue << 1);
- t4_defer_reply(skb, cdev, send_defer_abort_rpl);
- return;
- }
-
- set_abort_rpl_wr(reply_skb, GET_TID(req), status);
- kfree_skb(skb);
-
- set_wr_txq(reply_skb, CPL_PRIORITY_DATA, queue);
- if (csk_conn_inline(csk)) {
- struct l2t_entry *e = csk->l2t_entry;
-
- if (e && sk->sk_state != TCP_SYN_RECV) {
- cxgb4_l2t_send(csk->egress_dev, reply_skb, e);
- return;
- }
- }
- cxgb4_ofld_send(cdev->lldi->ports[0], reply_skb);
-}
-
static void chtls_send_abort_rpl(struct sock *sk, struct sk_buff *skb,
struct chtls_dev *cdev,
int status, int queue)
@@ -2079,8 +2046,8 @@ static void bl_abort_syn_rcv(struct sock *lsk, struct sk_buff *skb)
skb->sk = NULL;
do_abort_syn_rcv(child, lsk);
- send_abort_rpl(child, skb, BLOG_SKB_CB(skb)->cdev,
- CPL_ABORT_NO_RST, queue);
+ chtls_send_abort_rpl(child, skb, BLOG_SKB_CB(skb)->cdev,
+ CPL_ABORT_NO_RST, queue);
}
static int abort_syn_rcv(struct sock *sk, struct sk_buff *skb)
@@ -2111,7 +2078,7 @@ static int abort_syn_rcv(struct sock *sk, struct sk_buff *skb)
int queue = csk->txq_idx;
do_abort_syn_rcv(sk, psk);
- send_abort_rpl(sk, skb, cdev, CPL_ABORT_NO_RST, queue);
+ chtls_send_abort_rpl(sk, skb, cdev, CPL_ABORT_NO_RST, queue);
} else {
skb->sk = sk;
BLOG_SKB_CB(skb)->backlog_rcv = bl_abort_syn_rcv;
--
2.28.0.rc1.6.gae46588
^ permalink raw reply related [flat|nested] 9+ messages in thread* [PATCH net 2/7] chtls: Remove invalid set_tcb call
2021-01-06 4:29 [PATCH net 0/7] Bug fixes for chtls driver Ayush Sawal
2021-01-06 4:29 ` [PATCH net 1/7] chtls: Fix hardware tid leak Ayush Sawal
@ 2021-01-06 4:29 ` Ayush Sawal
2021-01-06 4:29 ` [PATCH net 3/7] chtls: Fix panic when route to peer not configured Ayush Sawal
` (5 subsequent siblings)
7 siblings, 0 replies; 9+ messages in thread
From: Ayush Sawal @ 2021-01-06 4:29 UTC (permalink / raw)
To: kuba, netdev, davem; +Cc: secdev, Ayush Sawal, Rohit Maheshwari
At the time of SYN_RECV, connection information is not
initialized at FW, updating tcb flag over uninitialized
connection causes adapter crash. We don't need to
update the flag during SYN_RECV state, so avoid this.
Fixes: cc35c88ae4db ("crypto : chtls - CPL handler definition")
Signed-off-by: Rohit Maheshwari <rohitm@chelsio.com>
Signed-off-by: Ayush Sawal <ayush.sawal@chelsio.com>
---
drivers/net/ethernet/chelsio/inline_crypto/chtls/chtls_cm.c | 3 ---
1 file changed, 3 deletions(-)
diff --git a/drivers/net/ethernet/chelsio/inline_crypto/chtls/chtls_cm.c b/drivers/net/ethernet/chelsio/inline_crypto/chtls/chtls_cm.c
index 561b5f2273af..431d1e3844ab 100644
--- a/drivers/net/ethernet/chelsio/inline_crypto/chtls/chtls_cm.c
+++ b/drivers/net/ethernet/chelsio/inline_crypto/chtls/chtls_cm.c
@@ -2096,9 +2096,6 @@ static void chtls_abort_req_rss(struct sock *sk, struct sk_buff *skb)
int queue = csk->txq_idx;
if (is_neg_adv(req->status)) {
- if (sk->sk_state == TCP_SYN_RECV)
- chtls_set_tcb_tflag(sk, 0, 0);
-
kfree_skb(skb);
return;
}
--
2.28.0.rc1.6.gae46588
^ permalink raw reply related [flat|nested] 9+ messages in thread* [PATCH net 3/7] chtls: Fix panic when route to peer not configured
2021-01-06 4:29 [PATCH net 0/7] Bug fixes for chtls driver Ayush Sawal
2021-01-06 4:29 ` [PATCH net 1/7] chtls: Fix hardware tid leak Ayush Sawal
2021-01-06 4:29 ` [PATCH net 2/7] chtls: Remove invalid set_tcb call Ayush Sawal
@ 2021-01-06 4:29 ` Ayush Sawal
2021-01-06 4:29 ` [PATCH net 4/7] chtls: Avoid unnecessary freeing of oreq pointer Ayush Sawal
` (4 subsequent siblings)
7 siblings, 0 replies; 9+ messages in thread
From: Ayush Sawal @ 2021-01-06 4:29 UTC (permalink / raw)
To: kuba, netdev, davem; +Cc: secdev, Ayush Sawal, Rohit Maheshwari
If route to peer is not configured, we might get non tls
devices from dst_neigh_lookup() which is invalid, adding a
check to avoid it.
Fixes: cc35c88ae4db ("crypto : chtls - CPL handler definition")
Signed-off-by: Rohit Maheshwari <rohitm@chelsio.com>
Signed-off-by: Ayush Sawal <ayush.sawal@chelsio.com>
---
.../chelsio/inline_crypto/chtls/chtls_cm.c | 14 ++++++++++++--
1 file changed, 12 insertions(+), 2 deletions(-)
diff --git a/drivers/net/ethernet/chelsio/inline_crypto/chtls/chtls_cm.c b/drivers/net/ethernet/chelsio/inline_crypto/chtls/chtls_cm.c
index 431d1e3844ab..04a8bd5af3b9 100644
--- a/drivers/net/ethernet/chelsio/inline_crypto/chtls/chtls_cm.c
+++ b/drivers/net/ethernet/chelsio/inline_crypto/chtls/chtls_cm.c
@@ -1109,6 +1109,7 @@ static struct sock *chtls_recv_sock(struct sock *lsk,
const struct cpl_pass_accept_req *req,
struct chtls_dev *cdev)
{
+ struct adapter *adap = pci_get_drvdata(cdev->pdev);
struct neighbour *n = NULL;
struct inet_sock *newinet;
const struct iphdr *iph;
@@ -1118,9 +1119,10 @@ static struct sock *chtls_recv_sock(struct sock *lsk,
struct dst_entry *dst;
struct tcp_sock *tp;
struct sock *newsk;
+ bool found = false;
u16 port_id;
int rxq_idx;
- int step;
+ int step, i;
iph = (const struct iphdr *)network_hdr;
newsk = tcp_create_openreq_child(lsk, oreq, cdev->askb);
@@ -1152,7 +1154,7 @@ static struct sock *chtls_recv_sock(struct sock *lsk,
n = dst_neigh_lookup(dst, &ip6h->saddr);
#endif
}
- if (!n)
+ if (!n || !n->dev)
goto free_sk;
ndev = n->dev;
@@ -1161,6 +1163,13 @@ static struct sock *chtls_recv_sock(struct sock *lsk,
if (is_vlan_dev(ndev))
ndev = vlan_dev_real_dev(ndev);
+ for_each_port(adap, i)
+ if (cdev->ports[i] == ndev)
+ found = true;
+
+ if (!found)
+ goto free_dst;
+
port_id = cxgb4_port_idx(ndev);
csk = chtls_sock_create(cdev);
@@ -1238,6 +1247,7 @@ static struct sock *chtls_recv_sock(struct sock *lsk,
free_csk:
chtls_sock_release(&csk->kref);
free_dst:
+ neigh_release(n);
dst_release(dst);
free_sk:
inet_csk_prepare_forced_close(newsk);
--
2.28.0.rc1.6.gae46588
^ permalink raw reply related [flat|nested] 9+ messages in thread* [PATCH net 4/7] chtls: Avoid unnecessary freeing of oreq pointer
2021-01-06 4:29 [PATCH net 0/7] Bug fixes for chtls driver Ayush Sawal
` (2 preceding siblings ...)
2021-01-06 4:29 ` [PATCH net 3/7] chtls: Fix panic when route to peer not configured Ayush Sawal
@ 2021-01-06 4:29 ` Ayush Sawal
2021-01-06 4:29 ` [PATCH net 5/7] chtls: Replace skb_dequeue with skb_peek Ayush Sawal
` (3 subsequent siblings)
7 siblings, 0 replies; 9+ messages in thread
From: Ayush Sawal @ 2021-01-06 4:29 UTC (permalink / raw)
To: kuba, netdev, davem; +Cc: secdev, Ayush Sawal, Rohit Maheshwari
In chtls_pass_accept_request(), removing the chtls_reqsk_free()
call to avoid oreq freeing twice. Here oreq is the pointer to
struct request_sock.
Fixes: cc35c88ae4db ("crypto : chtls - CPL handler definition")
Signed-off-by: Rohit Maheshwari <rohitm@chelsio.com>
Signed-off-by: Ayush Sawal <ayush.sawal@chelsio.com>
---
drivers/net/ethernet/chelsio/inline_crypto/chtls/chtls_cm.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/drivers/net/ethernet/chelsio/inline_crypto/chtls/chtls_cm.c b/drivers/net/ethernet/chelsio/inline_crypto/chtls/chtls_cm.c
index 04a8bd5af3b9..3022c802d09a 100644
--- a/drivers/net/ethernet/chelsio/inline_crypto/chtls/chtls_cm.c
+++ b/drivers/net/ethernet/chelsio/inline_crypto/chtls/chtls_cm.c
@@ -1397,7 +1397,7 @@ static void chtls_pass_accept_request(struct sock *sk,
newsk = chtls_recv_sock(sk, oreq, network_hdr, req, cdev);
if (!newsk)
- goto free_oreq;
+ goto reject;
if (chtls_get_module(newsk))
goto reject;
@@ -1413,8 +1413,6 @@ static void chtls_pass_accept_request(struct sock *sk,
kfree_skb(skb);
return;
-free_oreq:
- chtls_reqsk_free(oreq);
reject:
mk_tid_release(reply_skb, 0, tid);
cxgb4_ofld_send(cdev->lldi->ports[0], reply_skb);
--
2.28.0.rc1.6.gae46588
^ permalink raw reply related [flat|nested] 9+ messages in thread* [PATCH net 5/7] chtls: Replace skb_dequeue with skb_peek
2021-01-06 4:29 [PATCH net 0/7] Bug fixes for chtls driver Ayush Sawal
` (3 preceding siblings ...)
2021-01-06 4:29 ` [PATCH net 4/7] chtls: Avoid unnecessary freeing of oreq pointer Ayush Sawal
@ 2021-01-06 4:29 ` Ayush Sawal
2021-01-06 4:29 ` [PATCH net 6/7] chtls: Added a check to avoid NULL pointer dereference Ayush Sawal
` (2 subsequent siblings)
7 siblings, 0 replies; 9+ messages in thread
From: Ayush Sawal @ 2021-01-06 4:29 UTC (permalink / raw)
To: kuba, netdev, davem; +Cc: secdev, Ayush Sawal, Vinay Kumar Yadav
The skb is unlinked twice, one in __skb_dequeue in function
chtls_reset_synq() and another in cleanup_syn_rcv_conn().
So in this patch using skb_peek() instead of __skb_dequeue(),
so that unlink will be handled only in cleanup_syn_rcv_conn().
Fixes: cc35c88ae4db ("crypto : chtls - CPL handler definition")
Signed-off-by: Vinay Kumar Yadav <vinay.yadav@chelsio.com>
Signed-off-by: Ayush Sawal <ayush.sawal@chelsio.com>
---
drivers/net/ethernet/chelsio/inline_crypto/chtls/chtls_cm.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/chelsio/inline_crypto/chtls/chtls_cm.c b/drivers/net/ethernet/chelsio/inline_crypto/chtls/chtls_cm.c
index 3022c802d09a..ff3969a24d74 100644
--- a/drivers/net/ethernet/chelsio/inline_crypto/chtls/chtls_cm.c
+++ b/drivers/net/ethernet/chelsio/inline_crypto/chtls/chtls_cm.c
@@ -621,7 +621,7 @@ static void chtls_reset_synq(struct listen_ctx *listen_ctx)
while (!skb_queue_empty(&listen_ctx->synq)) {
struct chtls_sock *csk =
- container_of((struct synq *)__skb_dequeue
+ container_of((struct synq *)skb_peek
(&listen_ctx->synq), struct chtls_sock, synq);
struct sock *child = csk->sk;
--
2.28.0.rc1.6.gae46588
^ permalink raw reply related [flat|nested] 9+ messages in thread* [PATCH net 6/7] chtls: Added a check to avoid NULL pointer dereference
2021-01-06 4:29 [PATCH net 0/7] Bug fixes for chtls driver Ayush Sawal
` (4 preceding siblings ...)
2021-01-06 4:29 ` [PATCH net 5/7] chtls: Replace skb_dequeue with skb_peek Ayush Sawal
@ 2021-01-06 4:29 ` Ayush Sawal
2021-01-06 4:29 ` [PATCH net 7/7] chtls: Fix chtls resources release sequence Ayush Sawal
2021-01-08 1:20 ` [PATCH net 0/7] Bug fixes for chtls driver patchwork-bot+netdevbpf
7 siblings, 0 replies; 9+ messages in thread
From: Ayush Sawal @ 2021-01-06 4:29 UTC (permalink / raw)
To: kuba, netdev, davem; +Cc: secdev, Ayush Sawal, Vinay Kumar Yadav
In case of server removal lookup_stid() may return NULL pointer, which
is used as listen_ctx. So added a check before accessing this pointer.
Fixes: cc35c88ae4db ("crypto : chtls - CPL handler definition")
Signed-off-by: Vinay Kumar Yadav <vinay.yadav@chelsio.com>
Signed-off-by: Ayush Sawal <ayush.sawal@chelsio.com>
---
drivers/net/ethernet/chelsio/inline_crypto/chtls/chtls_cm.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/drivers/net/ethernet/chelsio/inline_crypto/chtls/chtls_cm.c b/drivers/net/ethernet/chelsio/inline_crypto/chtls/chtls_cm.c
index ff3969a24d74..1c6d3c93a0c8 100644
--- a/drivers/net/ethernet/chelsio/inline_crypto/chtls/chtls_cm.c
+++ b/drivers/net/ethernet/chelsio/inline_crypto/chtls/chtls_cm.c
@@ -1597,6 +1597,11 @@ static int chtls_pass_establish(struct chtls_dev *cdev, struct sk_buff *skb)
sk_wake_async(sk, 0, POLL_OUT);
data = lookup_stid(cdev->tids, stid);
+ if (!data) {
+ /* listening server close */
+ kfree_skb(skb);
+ goto unlock;
+ }
lsk = ((struct listen_ctx *)data)->lsk;
bh_lock_sock(lsk);
--
2.28.0.rc1.6.gae46588
^ permalink raw reply related [flat|nested] 9+ messages in thread* [PATCH net 7/7] chtls: Fix chtls resources release sequence
2021-01-06 4:29 [PATCH net 0/7] Bug fixes for chtls driver Ayush Sawal
` (5 preceding siblings ...)
2021-01-06 4:29 ` [PATCH net 6/7] chtls: Added a check to avoid NULL pointer dereference Ayush Sawal
@ 2021-01-06 4:29 ` Ayush Sawal
2021-01-08 1:20 ` [PATCH net 0/7] Bug fixes for chtls driver patchwork-bot+netdevbpf
7 siblings, 0 replies; 9+ messages in thread
From: Ayush Sawal @ 2021-01-06 4:29 UTC (permalink / raw)
To: kuba, netdev, davem; +Cc: secdev, Ayush Sawal, Vinay Kumar Yadav
CPL_ABORT_RPL is sent after releasing the resources by calling
chtls_release_resources(sk); and chtls_conn_done(sk);
eventually causing kernel panic. Fixing it by calling release
in appropriate order.
Fixes: cc35c88ae4db ("crypto : chtls - CPL handler definition")
Signed-off-by: Vinay Kumar Yadav <vinay.yadav@chelsio.com>
Signed-off-by: Ayush Sawal <ayush.sawal@chelsio.com>
---
.../net/ethernet/chelsio/inline_crypto/chtls/chtls_cm.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/net/ethernet/chelsio/inline_crypto/chtls/chtls_cm.c b/drivers/net/ethernet/chelsio/inline_crypto/chtls/chtls_cm.c
index 1c6d3c93a0c8..51dd030b3b36 100644
--- a/drivers/net/ethernet/chelsio/inline_crypto/chtls/chtls_cm.c
+++ b/drivers/net/ethernet/chelsio/inline_crypto/chtls/chtls_cm.c
@@ -2058,9 +2058,9 @@ static void bl_abort_syn_rcv(struct sock *lsk, struct sk_buff *skb)
queue = csk->txq_idx;
skb->sk = NULL;
- do_abort_syn_rcv(child, lsk);
chtls_send_abort_rpl(child, skb, BLOG_SKB_CB(skb)->cdev,
CPL_ABORT_NO_RST, queue);
+ do_abort_syn_rcv(child, lsk);
}
static int abort_syn_rcv(struct sock *sk, struct sk_buff *skb)
@@ -2090,8 +2090,8 @@ static int abort_syn_rcv(struct sock *sk, struct sk_buff *skb)
if (!sock_owned_by_user(psk)) {
int queue = csk->txq_idx;
- do_abort_syn_rcv(sk, psk);
chtls_send_abort_rpl(sk, skb, cdev, CPL_ABORT_NO_RST, queue);
+ do_abort_syn_rcv(sk, psk);
} else {
skb->sk = sk;
BLOG_SKB_CB(skb)->backlog_rcv = bl_abort_syn_rcv;
@@ -2135,12 +2135,12 @@ static void chtls_abort_req_rss(struct sock *sk, struct sk_buff *skb)
if (sk->sk_state == TCP_SYN_RECV && !abort_syn_rcv(sk, skb))
return;
- chtls_release_resources(sk);
- chtls_conn_done(sk);
}
chtls_send_abort_rpl(sk, skb, BLOG_SKB_CB(skb)->cdev,
rst_status, queue);
+ chtls_release_resources(sk);
+ chtls_conn_done(sk);
}
static void chtls_abort_rpl_rss(struct sock *sk, struct sk_buff *skb)
--
2.28.0.rc1.6.gae46588
^ permalink raw reply related [flat|nested] 9+ messages in thread* Re: [PATCH net 0/7] Bug fixes for chtls driver
2021-01-06 4:29 [PATCH net 0/7] Bug fixes for chtls driver Ayush Sawal
` (6 preceding siblings ...)
2021-01-06 4:29 ` [PATCH net 7/7] chtls: Fix chtls resources release sequence Ayush Sawal
@ 2021-01-08 1:20 ` patchwork-bot+netdevbpf
7 siblings, 0 replies; 9+ messages in thread
From: patchwork-bot+netdevbpf @ 2021-01-08 1:20 UTC (permalink / raw)
To: Ayush Sawal; +Cc: kuba, netdev, davem, secdev
Hello:
This series was applied to netdev/net.git (refs/heads/master):
On Wed, 6 Jan 2021 09:59:05 +0530 you wrote:
> patch 1: Fix hardware tid leak.
> patch 2: Remove invalid set_tcb call.
> patch 3: Fix panic when route to peer not configured.
> patch 4: Avoid unnecessary freeing of oreq pointer.
> patch 5: Replace skb_dequeue with skb_peek.
> patch 6: Added a check to avoid NULL pointer dereference patch.
> patch 7: Fix chtls resources release sequence.
>
> [...]
Here is the summary with links:
- [net,1/7] chtls: Fix hardware tid leak
https://git.kernel.org/netdev/net/c/717df0f4cdc9
- [net,2/7] chtls: Remove invalid set_tcb call
https://git.kernel.org/netdev/net/c/827d329105bf
- [net,3/7] chtls: Fix panic when route to peer not configured
https://git.kernel.org/netdev/net/c/5a5fac9966bb
- [net,4/7] chtls: Avoid unnecessary freeing of oreq pointer
https://git.kernel.org/netdev/net/c/f8d15d29d6e6
- [net,5/7] chtls: Replace skb_dequeue with skb_peek
https://git.kernel.org/netdev/net/c/a84b2c0d5fa2
- [net,6/7] chtls: Added a check to avoid NULL pointer dereference
https://git.kernel.org/netdev/net/c/eade1e0a4fb3
- [net,7/7] chtls: Fix chtls resources release sequence
https://git.kernel.org/netdev/net/c/15ef6b0e30b3
You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html
^ permalink raw reply [flat|nested] 9+ messages in thread