refcount leak in pep_sock_accept

[Hangyu Hua]

sock_hold(sk) is invoked in pep_sock_accept(),but __sock_put() is not invoked in subsequent failure branches(pep_accept_conn() != 0).

static struct sock *pep_sock_accept(struct sock *sk, int flags, int *errp,
        bool kern)
{
 struct pep_sock *pn = pep_sk(sk), *newpn;
 struct sock *newsk = NULL;
 struct sk_buff *skb;
 struct pnpipehdr *hdr;
 struct sockaddr_pn dst, src;
 int err;
 u16 peer_type;
 u8 pipe_handle, enabled, n_sb;
 u8 aligned = 0;
...
 newsk = sk_alloc(sock_net(sk), PF_PHONET, GFP_KERNEL, sk->sk_prot,
    kern);
 if (!newsk) {
  pep_reject_conn(sk, skb, PN_PIPE_ERR_OVERLOAD, GFP_KERNEL);
  err = -ENOBUFS;
  goto drop;
 }
...
 sock_hold(sk); <---- here,sk->sk_refcnt++
 newpn->listener = sk;
 skb_queue_head_init(&newpn->ctrlreq_queue);
 newpn->pipe_handle = pipe_handle;
 atomic_set(&newpn->tx_credits, 0);
 newpn->ifindex = 0;
 newpn->peer_type = peer_type;
 newpn->rx_credits = 0;
 newpn->rx_fc = newpn->tx_fc = PN_LEGACY_FLOW_CONTROL;
 newpn->init_enable = enabled;
 newpn->aligned = aligned;

 err = pep_accept_conn(newsk, skb);
 if (err) {
  sock_put(newsk); <---- before sock_put(newsk) may need sk->sk_refcnt--
  newsk = NULL;
  goto drop;
 }
 sk_add_node(newsk, &pn->hlist);
drop:
 release_sock(sk);
 kfree_skb(skb);
 *errp = err;
 return newsk;
}

My suggestion for the patch:

static struct sock *pep_sock_accept(struct sock *sk, int flags, int *errp,
        bool kern)
{
...
 err = pep_accept_conn(newsk, skb);
 if (err) {
+++ __sock_put(sk);
  sock_put(newsk);
  newsk = NULL;
  goto drop;
 }
 sk_add_node(newsk, &pn->hlist);
drop:
 release_sock(sk);
 kfree_skb(skb);
 *errp = err;
 return newsk;
}

[Seth Arnold (seth-arnold)]

Hello, can you please report this to <email address hidden>? That's the easiest way to get credit for finding and fixing the issue.

Thanks

[Hangyu Hua (hbh25y)]

Ok,i will report this to upstream kernel. Thank for your suggestion.

[Hangyu Hua (hbh25y)]

Hi, my patch was applied to netdev/net.git (master).

Here is the summary with links:
  - [net] Phonet: refcount leak in pep_sock_accep
    https://git.kernel.org/netdev/net/c/bcd0f9335332

By the way, can i get a CVE ID beacause of this?

[Seth Arnold (seth-arnold)]

Wonderful, thanks for getting back to us; you can request a CVE via this form: https://cveform.mitre.org/

Have you had any communication with the maintainers about when it may make it into Linus's tree? I'm not sure if the MITRE folks prefer for fixes to be in the kernel first, or if it's enough for them to be in the process.

Thanks

[Hangyu Hua (hbh25y)]

Thanks for your reply. I am not sure when linus will merge this to his branch. So i requested a reserved CVE via https://cveform.mitre.org/ yesterday. They haven't responded to me temporarily.

Can i request a CVE via ubuntu at next time?
I find ubuntu kernel in https://www.cve.org/PartnerInformation/ListofPartners. It seems more convenient to submit the bug to upstream kernel and ubuntu at the same time.

Thanks

[Seth Arnold (seth-arnold)]

While Ubuntu can assign CVEs for the kernel, it's not always in the best interests of the wider community for us to do so:

- we get a lot of requests for CVEs with code straight from syzkaller. We don't have the ability to assign CVEs for every issue from syzkaller, that's just not feasible for us to approach. syzkaller issues point out a problematic series of syscalls but do not identify the faulty code.

- it can happen that our initial impression of what is at fault is incorrect. Sometimes the fixes are the obvious one-liners and we're all very happy about these :) but sometimes problems require significant time to investigate and propose solutions. These are best handled in collaboration with upstream.

- so many people and organizations work on the Linux kernel that there's risk of multiple assignments. When we're the only ones who've been working on an issue, it's a low risk, but when multiple groups have been working on an issue, it's a higher risk. (It's not horrible consequences but it is annoying for everyone involved.)

The kernel upstream developers have asked us to forward reporters to them, so proper fixes can be identified quickly.

Other people who consume CVEs are best served by having specific fixes already available. A CVE to identify a problem is better than nothing, but best is having a solution already at hand.

So, this is why we ask people reporting kernel problems to work with upstream developers to identify fixes, and then once a fix is identified, to work on assigning a number. (The upstream developers don't care about CVE numbers, they don't want to be part of that process.)

It's a long way of saying that yes, we can assign numbers for issues in the Linux kernel, but before we do so, we should work with the upstream developers to identify fixes; and it's often better to have someone else assign the numbers, which is why we often ask reporters to use the CVE Form hosted by MITRE.

Thanks

[Hangyu Hua (hbh25y)]

I get it. I don't know things are so complex. I just thought it may be a better choice to request a CVE via a Linux distribution, and it seems to take a lot of time to request via https://cveform.mitre.org/.

To be honest, I don't care about CVE number either. But as a rookie security researcher, it seems important to know how to request one.:)

Thank you for taking the time to say this to me.

[Seth Arnold (seth-arnold)]

Part of the problem is there's just not enough people assigning numbers; it takes time to research an issue well enough to understand what the fault is, what the fix is, what versions of the software are affected, and distill that information into something that is useful for other people.

So, spreading it around does make a lot of sense. I'm sure I'm not the only one who wishes for more resources on assigning numbers and identifying issues and their fixes, etc.

Welcome aboard. :)

Thanks

[Thadeu Lima de Souza Cascardo (cascardo)]

Hi, Hangyu Hua.

Can you confirm that CVE-2021-45095 is what has been assigned to it? And that the upstream fix is now public and corresponds to commit https://git.kernel.org/pub/scm/linux/kernel/git/netdev/net.git/commit/?id=bcd0f93353326954817a4f9fa55ec57fb38acbb0 ?

Thank you very much.
Cascardo.

[Hangyu Hua (hbh25y)]

Hi

CVE id is right. The upstream kernerl fix is https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=bcd0f93353326954817a4f9fa55ec57fb38acbb0 now.

Thanks.

[Seth Arnold (seth-arnold)]

It looks like I lost track of this browser tab a lot longer than I expected.

Thanks Hangyu Hua for the fixes! :)

[Ubuntu Kernel Bot (ubuntu-kernel-bot)]

This bug is missing log files that will aid in diagnosing the problem. While running an Ubuntu kernel (not a mainline or third-party kernel) please enter the following command in a terminal window:

apport-collect 1953022

and then change the status of the bug to 'Confirmed'.

If, due to the nature of the issue you have encountered, you are unable to run this command, please add a comment stating that fact and change the bug status to 'Confirmed'.

This change has been made by an automated script, maintained by the Ubuntu Kernel Team.

[Launchpad Janitor (janitor)]

[Expired for linux (Ubuntu) because there has been no activity for 60 days.]