netdev - Re: [PATCH] net: fix memory leak in security_sk

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <CAKH8qBvKSV0Y5PuxiBwuOmyFFXMSZmOOQHSQ0LgvBXJDA+6xNw@mail.gmail.com>
Date:   Fri, 11 Nov 2022 10:52:54 -0800
From:   Stanislav Fomichev <sdf@...gle.com>
To:     Paul Moore <paul@...l-moore.com>
Cc:     Wang Yufen <wangyufen@...wei.com>,
        linux-security-module@...r.kernel.org, netdev@...r.kernel.org,
        jmorris@...ei.org, serge@...lyn.com, martin.lau@...nel.org,
        daniel@...earbox.net, ast@...nel.org, pabeni@...hat.com,
        kuba@...nel.org, edumazet@...gle.com
Subject: Re: [PATCH] net: fix memory leak in security_sk_alloc()

On Fri, Nov 11, 2022 at 7:08 AM Paul Moore <paul@...l-moore.com> wrote:
>
> On Fri, Nov 11, 2022 at 4:32 AM Wang Yufen <wangyufen@...wei.com> wrote:
> >
> > kmemleak reports this issue:
> >
> > unreferenced object 0xffff88810b7835c0 (size 32):
> >   comm "test_progs", pid 270, jiffies 4294969007 (age 1621.315s)
> >   hex dump (first 32 bytes):
> >     00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
> >     03 00 00 00 03 00 00 00 0f 00 00 00 00 00 00 00  ................
> >   backtrace:
> >     [<00000000376cdeab>] kmalloc_trace+0x27/0x110
> >     [<000000003bcdb3b6>] selinux_sk_alloc_security+0x66/0x110
> >     [<000000003959008f>] security_sk_alloc+0x47/0x80
> >     [<00000000e7bc6668>] sk_prot_alloc+0xbd/0x1a0
> >     [<0000000002d6343a>] sk_alloc+0x3b/0x940
> >     [<000000009812a46d>] unix_create1+0x8f/0x3d0
> >     [<000000005ed0976b>] unix_create+0xa1/0x150
> >     [<0000000086a1d27f>] __sock_create+0x233/0x4a0
> >     [<00000000cffe3a73>] __sys_socket_create.part.0+0xaa/0x110
> >     [<0000000007c63f20>] __sys_socket+0x49/0xf0
> >     [<00000000b08753c8>] __x64_sys_socket+0x42/0x50
> >     [<00000000b56e26b3>] do_syscall_64+0x3b/0x90
> >     [<000000009b4871b8>] entry_SYSCALL_64_after_hwframe+0x63/0xcd
> >
> > The issue occurs in the following scenarios:
> >
> > unix_create1()
> >   sk_alloc()
> >     sk_prot_alloc()
> >       security_sk_alloc()
> >         call_int_hook()
> >           hlist_for_each_entry()
> >             entry1->hook.sk_alloc_security
> >             <-- selinux_sk_alloc_security() succeeded,
> >             <-- sk->security alloced here.
> >             entry2->hook.sk_alloc_security
> >             <-- bpf_lsm_sk_alloc_security() failed
> >       goto out_free;
> >         ...    <-- the sk->security not freed, memleak
> >
> > To fix, if security_sk_alloc() failed and sk->security not null,
> > goto out_free_sec to reclaim resources.
> >
> > I'm not sure whether this fix makes sense, but if hook lists don't
> > support this usage, might need to modify the
> > "tools/testing/selftests/bpf/progs/lsm_cgroup.c" test case.
>
> The core problem is that the LSM is not yet fully stacked (work is
> actively going on in this space) which means that some LSM hooks do
> not support multiple LSMs at the same time; unfortunately the
> networking hooks fall into this category.
>
> While there can only be one LSM which manages the sock::sk_security
> field by defining a sk_alloc_security hook, it *should* be possible
> for other LSMs to to leverage the socket hooks, e.g.
> security_socket_bind(), as long as they don't manipulate any of the
> sock::sk_security state.
>
> I would suggest modifying the ".../bpf/progs/lsm_cgroup.c" test until
> the LSM supports stacking the networking hooks.

Agreed. Let's add some code to skip the test when it runs in the
environments that already have non-bpf lsms installed?

> --
> paul-moore.com