linux-kernel - Re: [PATCH v2 security-next 1/4] security: Hornet LSM

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: <CAADnVQJyNRZVLPj_nzegCyo+BzM1-whbnajotCXu+GW+5-=P6w@mail.gmail.com>
Date: Fri, 11 Apr 2025 17:09:14 -0700
From: Alexei Starovoitov <alexei.starovoitov@...il.com>
To: Blaise Boscaccy <bboscaccy@...ux.microsoft.com>
Cc: Jonathan Corbet <corbet@....net>, David Howells <dhowells@...hat.com>, 
	Herbert Xu <herbert@...dor.apana.org.au>, "David S. Miller" <davem@...emloft.net>, 
	Paul Moore <paul@...l-moore.com>, James Morris <jmorris@...ei.org>, 
	"Serge E. Hallyn" <serge@...lyn.com>, Masahiro Yamada <masahiroy@...nel.org>, 
	Nathan Chancellor <nathan@...nel.org>, Nicolas Schier <nicolas@...sle.eu>, Shuah Khan <shuah@...nel.org>, 
	Mickaël Salaün <mic@...ikod.net>, 
	Günther Noack <gnoack@...gle.com>, 
	Nick Desaulniers <nick.desaulniers+lkml@...il.com>, Bill Wendling <morbo@...gle.com>, 
	Justin Stitt <justinstitt@...gle.com>, Jarkko Sakkinen <jarkko@...nel.org>, 
	Jan Stancek <jstancek@...hat.com>, Neal Gompa <neal@...pa.dev>, 
	"open list:DOCUMENTATION" <linux-doc@...r.kernel.org>, LKML <linux-kernel@...r.kernel.org>, 
	keyrings@...r.kernel.org, 
	Linux Crypto Mailing List <linux-crypto@...r.kernel.org>, 
	LSM List <linux-security-module@...r.kernel.org>, 
	Linux Kbuild mailing list <linux-kbuild@...r.kernel.org>, 
	"open list:KERNEL SELFTEST FRAMEWORK" <linux-kselftest@...r.kernel.org>, bpf <bpf@...r.kernel.org>, 
	clang-built-linux <llvm@...ts.linux.dev>, nkapron@...gle.com, 
	Matteo Croce <teknoraver@...a.com>, Roberto Sassu <roberto.sassu@...wei.com>, 
	Cong Wang <xiyou.wangcong@...il.com>
Subject: Re: [PATCH v2 security-next 1/4] security: Hornet LSM

On Fri, Apr 4, 2025 at 2:56 PM Blaise Boscaccy
<bboscaccy@...ux.microsoft.com> wrote:
> +
> +static int hornet_find_maps(struct bpf_prog *prog, struct hornet_maps *maps)
> +{
> +       struct bpf_insn *insn = prog->insnsi;
> +       int insn_cnt = prog->len;
> +       int i;
> +       int err;
> +
> +       for (i = 0; i < insn_cnt; i++, insn++) {
> +               if (insn[0].code == (BPF_LD | BPF_IMM | BPF_DW)) {
> +                       switch (insn[0].src_reg) {
> +                       case BPF_PSEUDO_MAP_IDX_VALUE:
> +                       case BPF_PSEUDO_MAP_IDX:
> +                               err = add_used_map(maps, insn[0].imm);
> +                               if (err < 0)
> +                                       return err;
> +                               break;
> +                       default:
> +                               break;
> +                       }
> +               }
> +       }

...

> +               if (!map->frozen) {
> +                       attr.map_fd = fd;
> +                       err = kern_sys_bpf(BPF_MAP_FREEZE, &attr, sizeof(attr));

Sorry for the delay. Still swamped after conferences and the merge window.

Above are serious layering violations.
LSMs should not be looking that deep into bpf instructions.
Calling into sys_bpf from LSM is plain nack.

The verification of module signatures is a job of the module loading process.
The same thing should be done by the bpf system.
The signature needs to be passed into sys_bpf syscall
as a part of BPF_PROG_LOAD command.
It probably should be two new fields in union bpf_attr
(signature and length),
and the whole thing should be processed as part of the loading
with human readable error reported back through the verifier log
in case of signature mismatch, etc.

What LSM can do in addition is to say that if the signature is not
specified in the prog_load command then deny such request outright.
bpf syscall itself will deny program loading if signature is incorrect.