[PATCH mptcp-next] mptcp: Avoid NULL dereference in mptcp_getsockopt_subflow_addrs()

[PATCH mptcp-next] mptcp: Avoid NULL dereference in mptcp_getsockopt_subflow_addrs()

[Mat Martineau]

From: Tim Gardner <tim.gardner@canonical.com>

Coverity complains of a possible NULL dereference in
mptcp_getsockopt_subflow_addrs():

 861       } else if (sk->sk_family == AF_INET6) {
    	3. returned_null: inet6_sk returns NULL. [show details]
    	4. var_assigned: Assigning: np = NULL return value from inet6_sk.
 862                const struct ipv6_pinfo *np = inet6_sk(sk);

Fix this by checking for NULL.

Closes: https://github.com/multipath-tcp/mptcp_net-next/issues/231
Fixes: c11c5906bc0a ("mptcp: add MPTCP_SUBFLOW_ADDRS getsockopt support")
Cc: Florian Westphal <fw@strlen.de>
Signed-off-by: Tim Gardner <tim.gardner@canonical.com>
[mjm: Added WARN_ON_ONCE() to the unexpected case]
Signed-off-by: Mat Martineau <mathew.j.martineau@linux.intel.com>
---

This is a slight revision to Tim's patch submitted to netdev a few weeks ago:
https://lore.kernel.org/netdev/20210920154232.15494-1-tim.gardner@canonical.com/

I added the WARN_ON_ONCE() as Paolo suggested and added Closes:/Fixes:.

-Mat

---
 net/mptcp/sockopt.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/net/mptcp/sockopt.c b/net/mptcp/sockopt.c
index 8137cc3a4296..0f1e661c2032 100644
--- a/net/mptcp/sockopt.c
+++ b/net/mptcp/sockopt.c
@@ -861,6 +861,9 @@ static void mptcp_get_sub_addrs(const struct sock *sk, struct mptcp_subflow_addr
 	} else if (sk->sk_family == AF_INET6) {
 		const struct ipv6_pinfo *np = inet6_sk(sk);
 
+		if (WARN_ON_ONCE(!np))
+			return;
+
 		a->sin6_local.sin6_family = AF_INET6;
 		a->sin6_local.sin6_port = inet->inet_sport;
 
-- 
2.33.0

Re: [PATCH mptcp-next] mptcp: Avoid NULL dereference in mptcp_getsockopt_subflow_addrs()

[Matthieu Baerts]

Hi Mat, Tim,

On 12/10/2021 01:51, Mat Martineau wrote:
> From: Tim Gardner <tim.gardner@canonical.com>
> 
> Coverity complains of a possible NULL dereference in
> mptcp_getsockopt_subflow_addrs():
> 
>  861       } else if (sk->sk_family == AF_INET6) {
>     	3. returned_null: inet6_sk returns NULL. [show details]
>     	4. var_assigned: Assigning: np = NULL return value from inet6_sk.
>  862                const struct ipv6_pinfo *np = inet6_sk(sk);
> 
> Fix this by checking for NULL.
> 
> Closes: https://github.com/multipath-tcp/mptcp_net-next/issues/231
> Fixes: c11c5906bc0a ("mptcp: add MPTCP_SUBFLOW_ADDRS getsockopt support")
> Cc: Florian Westphal <fw@strlen.de>
> Signed-off-by: Tim Gardner <tim.gardner@canonical.com>
> [mjm: Added WARN_ON_ONCE() to the unexpected case]
> Signed-off-by: Mat Martineau <mathew.j.martineau@linux.intel.com>

Thank you for this patch, the review and the modifications!

Now in our tree: in "fixes for net", I guess that's what you wanted, right?

- 08dbe56922c1: mptcp: Avoid NULL dereference in mptcp_getsockopt_s(...)
- Results: 0c525f1e0da1..7d0aef192d20

Builds and tests are now in progress:



https://cirrus-ci.com/github/multipath-tcp/mptcp_net-next/export/20211014T121646

https://github.com/multipath-tcp/mptcp_net-next/actions/workflows/build-validation.yml?query=branch:export/20211014T121646

Cheers,
Matt
-- 
Tessares | Belgium | Hybrid Access Solutions
www.tessares.net