Return-Path: <linux-kernel-owner@vger.kernel.org>
Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand
	id S1755089AbbLDTEL (ORCPT <rfc822;w@1wt.eu>);
	Fri, 4 Dec 2015 14:04:11 -0500
Received: from mail-wm0-f47.google.com ([74.125.82.47]:33794 "EHLO
	mail-wm0-f47.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org
	with ESMTP id S1752606AbbLDTEI (ORCPT
	<rfc822;linux-kernel@vger.kernel.org>);
	Fri, 4 Dec 2015 14:04:08 -0500
MIME-Version: 1.0
In-Reply-To: <20151204184333.GA42737@ast-mbp.thefacebook.com>
References: <CACT4Y+ZxoR3UjLgcNdUm4fECLMx2VdtfrENMtRRCdgHB2n0bJA@mail.gmail.com>
 <20151204184333.GA42737@ast-mbp.thefacebook.com>
From: Dmitry Vyukov <dvyukov@google.com>
Date: Fri, 4 Dec 2015 20:03:47 +0100
Message-ID: <CACT4Y+YLV4scGXSFz=G3ACa-WE8VuT5wDGvPBV9D2Q+devxedA@mail.gmail.com>
Subject: Re: bpf: undefined shift in __bpf_prog_run
To: Alexei Starovoitov <alexei.starovoitov@gmail.com>
Cc: Alexei Starovoitov <ast@kernel.org>, netdev <netdev@vger.kernel.org>,
        LKML <linux-kernel@vger.kernel.org>,
        syzkaller <syzkaller@googlegroups.com>,
        Kostya Serebryany <kcc@google.com>,
        Alexander Potapenko <glider@google.com>,
        Sasha Levin <sasha.levin@oracle.com>,
        Eric Dumazet <edumazet@google.com>,
        Andrey Ryabinin <ryabinin.a.a@gmail.com>
Content-Type: text/plain; charset=UTF-8
Sender: linux-kernel-owner@vger.kernel.org
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org
Content-Length: 2353
Lines: 48

On Fri, Dec 4, 2015 at 7:43 PM, Alexei Starovoitov
<alexei.starovoitov@gmail.com> wrote:
> On Fri, Dec 04, 2015 at 12:17:08PM +0100, Dmitry Vyukov wrote:
>> Hello,
>>
>> UBSAN reports the following undefined behavior:
>>
>> UBSAN: Undefined behaviour in kernel/bpf/core.c:336:2
>> shift exponent 2835 is to large for 32-bit type 'unsigned int'
>> CPU: 1 PID: 14227 Comm: syzkaller_execu Not tainted 4.4.0-rc3+ #142
>> Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Bochs 01/01/2011
>>  0000000000000001 ffff88003892f898 ffffffff82c747b8 0000000041b58ab3
>>  ffffffff878cbc05 ffffffff82c74706 ffff88003892f860 ffff88003892f9a0
>>  0000000000000000 0000000000000b13 ffffffff88178de2 0000000000000020
>> Call Trace:
>>  [<ffffffff82d684f0>] __ubsan_handle_shift_out_of_bounds+0x294/0x2e5
>> lib/ubsan.c:417
>>  [<ffffffff8160a408>] __bpf_prog_run+0x8f48/0x9ac0 kernel/bpf/core.c:336
>>  [<     inline     >] seccomp_run_filters kernel/seccomp.c:198
>>  [<     inline     >] __seccomp_phase1_filter kernel/seccomp.c:588
>>  [<ffffffff8156ddfb>] seccomp_phase1+0x1cb/0x990 kernel/seccomp.c:667
>>  [<ffffffff8100651f>] syscall_trace_enter_phase1+0x28f/0x4e0
>> arch/x86/entry/common.c:132
>>  [<ffffffff8691b939>] tracesys+0xd/0x44 arch/x86/entry/entry_64.S:240
>
> is it with some random seccomp program?
> If normal libseccomp generates such programs than it needs to be fixed.

Yes, it is with completely random seccomp program.

>> Such shifts have undefined behavior according to C standard and behave
>> differently on different archs. I guess we don't want to rely on any
>> kind of undefined behavior in bpf/seccomp. And generally want to
>> completely define results of all operations in bpf.
>
> bpf is an engine and we're not going to slow down each shift operation
> by extra run-time checks or masks.
> In other words bpf shift instruction == shift in C. Both undefined
> with for large operands.
> If seccomp is relying on undefined behavior, it should be fixed.

But note that it is not that result of such operation is undefined, it
is overall kernel behavior that becomes undefined.
--
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/