Message-ID: <5542242D.1090502@iogearbox.net>
Date: Thu, 30 Apr 2015 14:46:37 +0200
From: Daniel Borkmann <daniel@iogearbox.net>
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.4.0
MIME-Version: 1.0
To: Nicolas Schichan <nschichan@freebox.fr>, Kees Cook <keescook@chromium.org>,
        Andy Lutomirski <luto@amacapital.net>, Will Drewry <wad@chromium.org>,
        linux-kernel@vger.kernel.org, ast@plumgrid.com, davem@davemloft.net
Subject: Re: [PATCH 2/4] seccomp: rework seccomp_prepare_filter().
References: <1430314657-2552-1-git-send-email-nschichan@freebox.fr> <1430314657-2552-3-git-send-email-nschichan@freebox.fr> <5541110D.4020507@iogearbox.net> <55421FA5.2070906@freebox.fr>
In-Reply-To: <55421FA5.2070906@freebox.fr>
Content-Type: text/plain; charset=windows-1252; format=flowed
Content-Transfer-Encoding: 7bit
Sender: linux-kernel-owner@vger.kernel.org
Content-Length: 2280
Lines: 83

On 04/30/2015 02:27 PM, Nicolas Schichan wrote:
...
> I'll take more care about the receiver list for the v2 of this serie.

Ok, cool.

>> I see, you need that to make it available to the old bpf_jit_compile()
>> for probing on classic JITs. Actually, I really would prefer, if instead
>> of duplicating that code, you could export bpf_prepare_filter() and
>> pass seccomp_check_filter() as an argument to bpf_prepare_filter().
>
> Just to be sure you want me to pass a pointer to seccomp_check_filter to
> bpf_prepare_filter so that it can run it between bpf_check_classic() and
> bpf_jit_compile ?

For example, what comes to mind is something along these lines:

struct bpf_prog *
bpf_prepare_filter(struct bpf_prog *fp,
		   int (*aux_trans_classic)(struct sock_filter *filter,
					    unsigned int flen))
{
	int err;

	fp->bpf_func = NULL;
	fp->jited = false;

	err = bpf_check_classic(fp->insns, fp->len);
	if (err) {
		__bpf_prog_release(fp);
		return ERR_PTR(err);
	}

	/* There might be additional checks and transformations
	 * needed on classic filters, f.e. in case of seccomp.
	 */
	if (aux_trans_classic) {
		err = aux_trans_classic(fp->insns,
					fp->len);
		if (err) {
			__bpf_prog_release(fp);
			return ERR_PTR(err);
		}
	}

	/* Probe if we can JIT compile the filter and if so, do
	 * the compilation of the filter.
	 */
	bpf_jit_compile(fp);

	/* JIT compiler couldn't process this filter, so do the
	 * internal BPF translation for the optimized interpreter.
	 */
	if (!fp->jited)
		fp = bpf_migrate_filter(fp);

	return fp;
}

 From seccomp side, you invoke:

... bpf_prepare_filter(fp, seccomp_check_filter);

I would actually like to see that customized code in seccomp_prepare_filter()
further reduced instead of increased, would certainly make it easier to
maintain and understand.

Do you think something like the above is possible?

>> Otherwise, in case bpf_prepare_filter() changes, people will easily
>> forget to update seccomp related code, really.
>
> Fair point.
>
> Thanks,

Thanks a lot,
Daniel
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/