linux-kernel - Re: [PATCH v2 net-next 2/4] net: add skeleton of bpfilter kernel module

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <20180509022928.elyxj6mohm2jud56@ast-mbp>
Date:   Tue, 8 May 2018 19:29:29 -0700
From:   Alexei Starovoitov <alexei.starovoitov@...il.com>
To:     "Luis R. Rodriguez" <mcgrof@...nel.org>
Cc:     Alexei Starovoitov <ast@...nel.org>, davem@...emloft.net,
        daniel@...earbox.net, torvalds@...ux-foundation.org,
        gregkh@...uxfoundation.org, luto@...capital.net,
        netdev@...r.kernel.org, linux-kernel@...r.kernel.org,
        kernel-team@...com, Juergen Gross <jgross@...e.com>,
        Eric Paris <eparis@...hat.com>,
        Matthew Auld <matthew.auld@...el.com>,
        Josh Triplett <josh@...htriplett.org>,
        "Kirill A. Shutemov" <kirill.shutemov@...ux.intel.com>,
        Joonas Lahtinen <joonas.lahtinen@...ux.intel.com>,
        Chris Wilson <chris@...is-wilson.co.uk>,
        Stephen Smalley <sds@...ho.nsa.gov>,
        "Eric W. Biederman" <ebiederm@...ssion.com>,
        Mimi Zohar <zohar@...ux.vnet.ibm.com>,
        David Howells <dhowells@...hat.com>,
        Kees Cook <keescook@...omium.org>,
        Andrew Morton <akpm@...ux-foundation.org>,
        Dominik Brodowski <linux@...inikbrodowski.net>,
        Hugh Dickins <hughd@...gle.com>, Jann Horn <jannh@...gle.com>,
        Jani Nikula <jani.nikula@...ux.intel.com>,
        Rodrigo Vivi <rodrigo.vivi@...el.com>,
        David Airlie <airlied@...ux.ie>,
        "Rafael J. Wysocki" <rjw@...ysocki.net>,
        Linux FS Devel <linux-fsdevel@...r.kernel.org>,
        pjones@...hat.com, Michael Matz <matz@...e.de>, mjg59@...gle.com,
        linux-security-module <linux-security-module@...r.kernel.org>,
        linux-integrity@...r.kernel.org
Subject: Re: [PATCH v2 net-next 2/4] net: add skeleton of bpfilter kernel
 module

On Mon, May 07, 2018 at 06:51:24PM +0000, Luis R. Rodriguez wrote:
> > Notice that _binary_net_bpfilter_bpfilter_umh_start - end
> > is placed into .init.rodata section, so it's freed as soon as __init
> > function of bpfilter.ko is finished.
> > As part of __init the bpfilter.ko does first request/reply action
> > via two unix pipe provided by fork_usermode_blob() helper to
> > make sure that umh is healthy. If not it will kill it via pid.
> 
> It does this very fast, right away. On a really slow system how are you sure
> that this won't race and the execution of the check happens early on prior to
> letting the actual setup trigger? After all, we're calling the userpsace
> process in async mode. We could preempt it now.

I don't see an issue.
the kernel synchronously writes into a pipe. User space process reads.
Exactly the same as coredump logic with pipes.

> > +# a bit of elf magic to convert bpfilter_umh binary into a binary blob
> > +# inside bpfilter_umh.o elf file referenced by
> > +# _binary_net_bpfilter_bpfilter_umh_start symbol
> > +# which bpfilter_kern.c passes further into umh blob loader at run-time
> > +quiet_cmd_copy_umh = GEN $@
> > +      cmd_copy_umh = echo ':' > $(obj)/.bpfilter_umh.o.cmd; \
> > +      $(OBJCOPY) -I binary -O $(CONFIG_OUTPUT_FORMAT) \
> > +      -B `$(OBJDUMP) -f $<|grep architecture|cut -d, -f1|cut -d' ' -f2` \
> > +      --rename-section .data=.init.rodata $< $@
> 
> Cool, but so our expectation is that the compiler sets this symbol, how
> are we sure it will always be set?

Compiler doesn't set it. objcopy does.

> > +
> > +	if (__bpfilter_process_sockopt(NULL, 0, 0, 0, 0) != 0) {
> 
> See, here, what if the userspace process gets preemtped and we run this
> check afterwards? Is that possible?

User space is a normal task. It can sleep and can be single stepped with GDB.