Message ID | 20201025194935.31754-1-vinay.yadav@chelsio.com |
---|---|
State | New |
Headers | show |
Series | [net,v2] chelsio/chtls: fix memory leaks | expand |
On Mon, 26 Oct 2020 01:19:36 +0530 Vinay Kumar Yadav wrote: > Correct skb refcount in alloc_ctrl_skb(), causing skb memleak > when chtls_send_abort() called with NULL skb. > Also race between user context and softirq causing memleak, > consider the call sequence scenario Sounds like two separate fixes? > chtls_setkey() //user context > chtls_peer_close() > chtls_abort_req_rss() > chtls_setkey() //user context > > work request skb queued in chtls_setkey() won't be freed > because resources are already cleaned for this connection, > fix it by not queuing work request while socket is closing. > > v1->v2: > - fix W=1 warning. > > Fixes: cc35c88ae4db ("crypto : chtls - CPL handler definition") > Signed-off-by: Vinay Kumar Yadav <vinay.yadav@chelsio.com> > --- > drivers/net/ethernet/chelsio/inline_crypto/chtls/chtls_cm.c | 2 +- > drivers/net/ethernet/chelsio/inline_crypto/chtls/chtls_hw.c | 3 +++ > 2 files changed, 4 insertions(+), 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 24154816d1d1..63aacc184f68 100644 > --- a/drivers/net/ethernet/chelsio/inline_crypto/chtls/chtls_cm.c > +++ b/drivers/net/ethernet/chelsio/inline_crypto/chtls/chtls_cm.c > @@ -212,7 +212,7 @@ static struct sk_buff *alloc_ctrl_skb(struct sk_buff *skb, int len) > { > if (likely(skb && !skb_shared(skb) && !skb_cloned(skb))) { > __skb_trim(skb, 0); > - refcount_add(2, &skb->users); > + refcount_inc(&skb->users); You just switched from adding two refs to adding one. Was this always leaking the skb? Also skb_get(). > } else { > skb = alloc_skb(len, GFP_KERNEL | __GFP_NOFAIL); > } > diff --git a/drivers/net/ethernet/chelsio/inline_crypto/chtls/chtls_hw.c b/drivers/net/ethernet/chelsio/inline_crypto/chtls/chtls_hw.c > index f1820aca0d33..62c829023da5 100644 > --- a/drivers/net/ethernet/chelsio/inline_crypto/chtls/chtls_hw.c > +++ b/drivers/net/ethernet/chelsio/inline_crypto/chtls/chtls_hw.c > @@ -383,6 +383,9 @@ int chtls_setkey(struct chtls_sock *csk, u32 keylen, > if (ret) > goto out_notcb; > > + if (unlikely(csk_flag(sk, CSK_ABORT_SHUTDOWN))) > + goto out_notcb; > + > set_wr_txq(skb, CPL_PRIORITY_DATA, csk->tlshws.txqid); > csk->wr_credits -= DIV_ROUND_UP(len, 16); > csk->wr_unacked += DIV_ROUND_UP(len, 16);
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 24154816d1d1..63aacc184f68 100644 --- a/drivers/net/ethernet/chelsio/inline_crypto/chtls/chtls_cm.c +++ b/drivers/net/ethernet/chelsio/inline_crypto/chtls/chtls_cm.c @@ -212,7 +212,7 @@ static struct sk_buff *alloc_ctrl_skb(struct sk_buff *skb, int len) { if (likely(skb && !skb_shared(skb) && !skb_cloned(skb))) { __skb_trim(skb, 0); - refcount_add(2, &skb->users); + refcount_inc(&skb->users); } else { skb = alloc_skb(len, GFP_KERNEL | __GFP_NOFAIL); } diff --git a/drivers/net/ethernet/chelsio/inline_crypto/chtls/chtls_hw.c b/drivers/net/ethernet/chelsio/inline_crypto/chtls/chtls_hw.c index f1820aca0d33..62c829023da5 100644 --- a/drivers/net/ethernet/chelsio/inline_crypto/chtls/chtls_hw.c +++ b/drivers/net/ethernet/chelsio/inline_crypto/chtls/chtls_hw.c @@ -383,6 +383,9 @@ int chtls_setkey(struct chtls_sock *csk, u32 keylen, if (ret) goto out_notcb; + if (unlikely(csk_flag(sk, CSK_ABORT_SHUTDOWN))) + goto out_notcb; + set_wr_txq(skb, CPL_PRIORITY_DATA, csk->tlshws.txqid); csk->wr_credits -= DIV_ROUND_UP(len, 16); csk->wr_unacked += DIV_ROUND_UP(len, 16);
Correct skb refcount in alloc_ctrl_skb(), causing skb memleak when chtls_send_abort() called with NULL skb. Also race between user context and softirq causing memleak, consider the call sequence scenario chtls_setkey() //user context chtls_peer_close() chtls_abort_req_rss() chtls_setkey() //user context work request skb queued in chtls_setkey() won't be freed because resources are already cleaned for this connection, fix it by not queuing work request while socket is closing. v1->v2: - fix W=1 warning. Fixes: cc35c88ae4db ("crypto : chtls - CPL handler definition") Signed-off-by: Vinay Kumar Yadav <vinay.yadav@chelsio.com> --- drivers/net/ethernet/chelsio/inline_crypto/chtls/chtls_cm.c | 2 +- drivers/net/ethernet/chelsio/inline_crypto/chtls/chtls_hw.c | 3 +++ 2 files changed, 4 insertions(+), 1 deletion(-)