Static analysis - possible NULL dereference in mptcp_get_sub_addrs()

[mjmartineau]

As reported on the mptcp and netdev mailing lists by @rtg-canonical:

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);

This may be a false positive by the static checker. The main question is whether sk_fullsock() can ever return NULL in this part of the code. For this to happen, the subflow would have to be on the msk->conn_list even though a subflow disconnect was initiated locally. This doesn't look like it's expected, but need to confirm that it's impossible.

[pabeni]

I double checked the relevant code paths and I think !np is not a possible condition in mptcp_get_sub_addrs().

Additionally, the address returned to user-space is zeroed at the beginning of such function, and I think that silencing the static checker is cheap and good, so Tim's patch looks correct to me:

https://marc.info/?l=linux-netdev&m=163215255522383&w=2

I would simply add a WARN_ON_ONCE() to document that event is really unexpected and get notified if the above statements prove to be uncorrect.:

        if (WARN_ON_ONCE(!np))
                return;