2022-02-15 14:44:04

by Jinmeng Zhou

[permalink] [raw]
Subject: 4 missing check bugs

Dear maintainers,

Hi, our tool finds several missing check bugs on
Linux kernel v4.18.5 using static analysis.
We are looking forward to having more experts' eyes on this. Thank you!

Before calling sk_alloc() with SOCK_RAW type,
there should be a permission check, ns_capable(ns,CAP_NET_RAW).
For example,

static int xsk_create(struct net *net, struct socket *sock, int protocol,
int kern)
{
struct xdp_sock *xs;
struct sock *sk;

if (!ns_capable(net->user_ns, CAP_NET_RAW))
return -EPERM;
if (sock->type != SOCK_RAW)
return -ESOCKTNOSUPPORT;
...
sk = sk_alloc(net, PF_XDP, GFP_KERNEL, &xsk_proto, kern);
if (!sk)
return -ENOBUFS;
...
}



We find 4 missing check bugs.
The functions that miss permission checks in v4.18.5:

net/bluetooth/hidp/sock.c
static int hidp_sock_create(struct net *net, struct socket *sock, int protocol,
int kern)
{
struct sock *sk;

BT_DBG("sock %p", sock);

if (sock->type != SOCK_RAW)
return -ESOCKTNOSUPPORT;

sk = sk_alloc(net, PF_BLUETOOTH, GFP_ATOMIC, &hidp_proto, kern);
if (!sk)
return -ENOMEM;
...
}


net/bluetooth/cmtp/sock.c
static int cmtp_sock_create(struct net *net, struct socket *sock, int protocol,
int kern)
{
struct sock *sk;

BT_DBG("sock %p", sock);

if (sock->type != SOCK_RAW)
return -ESOCKTNOSUPPORT;

sk = sk_alloc(net, PF_BLUETOOTH, GFP_ATOMIC, &cmtp_proto, kern);
if (!sk)
return -ENOMEM;
...
}


net/bluetooth/hci_sock.c
static int hci_sock_create(struct net *net, struct socket *sock, int protocol,
int kern)
{
struct sock *sk;

BT_DBG("sock %p", sock);

if (sock->type != SOCK_RAW)
return -ESOCKTNOSUPPORT;

sock->ops = &hci_sock_ops;

sk = sk_alloc(net, PF_BLUETOOTH, GFP_ATOMIC, &hci_sk_proto, kern);
if (!sk)
return -ENOMEM;
...
}


/net/bluetooth/bnep/sock.c
static int bnep_sock_create(struct net *net, struct socket *sock, int protocol,
int kern)
{
struct sock *sk;

BT_DBG("sock %p", sock);

if (sock->type != SOCK_RAW)
return -ESOCKTNOSUPPORT;

sk = sk_alloc(net, PF_BLUETOOTH, GFP_ATOMIC, &bnep_proto, kern);
if (!sk)
return -ENOMEM;
...
}


Thanks again!


Best regards,
Jinmeng Zhou


2022-02-15 23:30:07

by Eric Dumazet

[permalink] [raw]
Subject: Re: 4 missing check bugs


On 2/15/22 04:37, Jinmeng Zhou wrote:
> Dear maintainers,
>
> Hi, our tool finds several missing check bugs on
> Linux kernel v4.18.5 using static analysis.
> We are looking forward to having more experts' eyes on this. Thank you!
>
> Before calling sk_alloc() with SOCK_RAW type,
> there should be a permission check, ns_capable(ns,CAP_NET_RAW).
> For example,


v4.18 is not a stable kernel.

No one is supposed to use v4.18.5, and expect others to fix bugs in it.


2022-02-16 06:57:06

by Jinmeng Zhou

[permalink] [raw]
Subject: Re: 4 missing check bugs

Dear maintainers,

I double-check the newest version v5.17 and the stable version 5.16.9.
The code of the 4 functions remains the same.
Are they missing check bugs?

Thanks!


Best regards,
Jinmeng Zhou

On Wed, Feb 16, 2022 at 5:51 AM Eric Dumazet <[email protected]> wrote:
>
>
> On 2/15/22 04:37, Jinmeng Zhou wrote:
> > Dear maintainers,
> >
> > Hi, our tool finds several missing check bugs on
> > Linux kernel v4.18.5 using static analysis.
> > We are looking forward to having more experts' eyes on this. Thank you!
> >
> > Before calling sk_alloc() with SOCK_RAW type,
> > there should be a permission check, ns_capable(ns,CAP_NET_RAW).
> > For example,
>
>
> v4.18 is not a stable kernel.
>
> No one is supposed to use v4.18.5, and expect others to fix bugs in it.
>
>

2022-02-16 10:25:24

by Marcel Holtmann

[permalink] [raw]
Subject: Re: 4 missing check bugs

Hi Jinmeng,

> Hi, our tool finds several missing check bugs on
> Linux kernel v4.18.5 using static analysis.
> We are looking forward to having more experts' eyes on this. Thank you!
>
> Before calling sk_alloc() with SOCK_RAW type,
> there should be a permission check, ns_capable(ns,CAP_NET_RAW).
> For example,

says who? The appropriate checks are actually present, just not at sock_create. Some are at sock_bind.

Regards

Marcel