2020-04-25 12:59:15

by Xiyu Yang

[permalink] [raw]
Subject: [PATCH] net/tls: Fix sk_psock refcnt leak in bpf_exec_tx_verdict()

bpf_exec_tx_verdict() invokes sk_psock_get(), which returns a reference
of the specified sk_psock object to "psock" with increased refcnt.

When bpf_exec_tx_verdict() returns, local variable "psock" becomes
invalid, so the refcount should be decreased to keep refcount balanced.

The reference counting issue happens in one exception handling path of
bpf_exec_tx_verdict(). When "policy" equals to NULL but "psock" is not
NULL, the function forgets to decrease the refcnt increased by
sk_psock_get(), causing a refcnt leak.

Fix this issue by calling sk_psock_put() on this error path before
bpf_exec_tx_verdict() returns.

Signed-off-by: Xiyu Yang <[email protected]>
Signed-off-by: Xin Tan <[email protected]>
---
net/tls/tls_sw.c | 2 ++
1 file changed, 2 insertions(+)

diff --git a/net/tls/tls_sw.c b/net/tls/tls_sw.c
index 7d3bf86e6cbf..5fad144edaa3 100644
--- a/net/tls/tls_sw.c
+++ b/net/tls/tls_sw.c
@@ -800,6 +800,8 @@ static int bpf_exec_tx_verdict(struct sk_msg *msg, struct sock *sk,
*copied -= sk_msg_free(sk, msg);
tls_free_open_rec(sk);
}
+ if (psock)
+ sk_psock_put(sk, psock);
return err;
}
more_data:
--
2.7.4


2020-04-25 18:51:18

by Markus Elfring

[permalink] [raw]
Subject: Re: [PATCH] net/tls: Fix sk_psock refcnt leak in bpf_exec_tx_verdict()

> When bpf_exec_tx_verdict() returns, local variable "psock" becomes
> invalid, so the refcount should be decreased to keep refcount balanced.

How do you think about to mention the term “reference counting” in
the commit message?

Would you like to add the tag “Fixes” to the change description?

Regards,
Markus

2020-04-27 18:21:50

by David Miller

[permalink] [raw]
Subject: Re: [PATCH] net/tls: Fix sk_psock refcnt leak in bpf_exec_tx_verdict()

From: Xiyu Yang <[email protected]>
Date: Sat, 25 Apr 2020 20:54:37 +0800

> bpf_exec_tx_verdict() invokes sk_psock_get(), which returns a reference
> of the specified sk_psock object to "psock" with increased refcnt.
>
> When bpf_exec_tx_verdict() returns, local variable "psock" becomes
> invalid, so the refcount should be decreased to keep refcount balanced.
>
> The reference counting issue happens in one exception handling path of
> bpf_exec_tx_verdict(). When "policy" equals to NULL but "psock" is not
> NULL, the function forgets to decrease the refcnt increased by
> sk_psock_get(), causing a refcnt leak.
>
> Fix this issue by calling sk_psock_put() on this error path before
> bpf_exec_tx_verdict() returns.
>
> Signed-off-by: Xiyu Yang <[email protected]>
> Signed-off-by: Xin Tan <[email protected]>

Applied and queued up for -stable.