All of lore.kernel.org
 help / color / mirror / Atom feed
From: Alexei Starovoitov <alexei.starovoitov@gmail.com>
To: Kees Cook <keescook@chromium.org>
Cc: Yonghong Song <yhs@fb.com>, Dmitry Vyukov <dvyukov@google.com>,
	Kurt Manucredo <fuzzybritches0@gmail.com>,
	syzbot+bed360704c521841c85d@syzkaller.appspotmail.com,
	Andrii Nakryiko <andrii@kernel.org>,
	Alexei Starovoitov <ast@kernel.org>, bpf <bpf@vger.kernel.org>,
	Daniel Borkmann <daniel@iogearbox.net>,
	"David S. Miller" <davem@davemloft.net>,
	Jesper Dangaard Brouer <hawk@kernel.org>,
	John Fastabend <john.fastabend@gmail.com>,
	Martin KaFai Lau <kafai@fb.com>, KP Singh <kpsingh@kernel.org>,
	Jakub Kicinski <kuba@kernel.org>,
	LKML <linux-kernel@vger.kernel.org>,
	Network Development <netdev@vger.kernel.org>,
	Song Liu <songliubraving@fb.com>,
	syzkaller-bugs <syzkaller-bugs@googlegroups.com>,
	nathan@kernel.org, Nick Desaulniers <ndesaulniers@google.com>,
	Clang-Built-Linux ML <clang-built-linux@googlegroups.com>,
	linux-kernel-mentees@lists.linuxfoundation.org,
	Shuah Khan <skhan@linuxfoundation.org>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	Kernel Hardening <kernel-hardening@lists.openwall.com>,
	kasan-dev <kasan-dev@googlegroups.com>
Subject: Re: [PATCH v4] bpf: core: fix shift-out-of-bounds in ___bpf_prog_run
Date: Thu, 10 Jun 2021 10:52:37 -0700	[thread overview]
Message-ID: <CAADnVQKMwKYgthoQV4RmGpZm9Hm-=wH3DoaNqs=UZRmJKefwGw@mail.gmail.com> (raw)
In-Reply-To: <202106101002.DF8C7EF@keescook>

On Thu, Jun 10, 2021 at 10:06 AM Kees Cook <keescook@chromium.org> wrote:
>
> > > I guess the main question: what should happen if a bpf program writer
> > > does _not_ use compiler nor check_shl_overflow()?
>
> I think the BPF runtime needs to make such actions defined, instead of
> doing a blind shift. It needs to check the size of the shift explicitly
> when handling the shift instruction.

Such ideas were brought up in the past and rejected.
We're not going to sacrifice performance to make behavior a bit more
'defined'. CPUs are doing it deterministically. It's the C standard
that needs fixing.

> Sure, but the point of UBSAN is to find and alert about undefined
> behavior, so we still need to fix this.

No. The undefined behavior of C standard doesn't need "fixing" most of the time.

WARNING: multiple messages have this Message-ID (diff)
From: Alexei Starovoitov <alexei.starovoitov@gmail.com>
To: Kees Cook <keescook@chromium.org>
Cc: Song Liu <songliubraving@fb.com>,
	Kernel Hardening <kernel-hardening@lists.openwall.com>,
	Alexei Starovoitov <ast@kernel.org>,
	Andrii Nakryiko <andrii@kernel.org>,
	syzbot+bed360704c521841c85d@syzkaller.appspotmail.com,
	Daniel Borkmann <daniel@iogearbox.net>,
	John Fastabend <john.fastabend@gmail.com>,
	kasan-dev <kasan-dev@googlegroups.com>,
	Clang-Built-Linux ML <clang-built-linux@googlegroups.com>,
	Jakub Kicinski <kuba@kernel.org>,
	linux-kernel-mentees@lists.linuxfoundation.org,
	Jesper Dangaard Brouer <hawk@kernel.org>,
	syzkaller-bugs <syzkaller-bugs@googlegroups.com>,
	KP Singh <kpsingh@kernel.org>,
	nathan@kernel.org, Yonghong Song <yhs@fb.com>,
	Dmitry Vyukov <dvyukov@google.com>,
	Network Development <netdev@vger.kernel.org>,
	Nick Desaulniers <ndesaulniers@google.com>,
	LKML <linux-kernel@vger.kernel.org>,
	"David S. Miller" <davem@davemloft.net>,
	bpf <bpf@vger.kernel.org>, Martin KaFai Lau <kafai@fb.com>
Subject: Re: [PATCH v4] bpf: core: fix shift-out-of-bounds in ___bpf_prog_run
Date: Thu, 10 Jun 2021 10:52:37 -0700	[thread overview]
Message-ID: <CAADnVQKMwKYgthoQV4RmGpZm9Hm-=wH3DoaNqs=UZRmJKefwGw@mail.gmail.com> (raw)
In-Reply-To: <202106101002.DF8C7EF@keescook>

On Thu, Jun 10, 2021 at 10:06 AM Kees Cook <keescook@chromium.org> wrote:
>
> > > I guess the main question: what should happen if a bpf program writer
> > > does _not_ use compiler nor check_shl_overflow()?
>
> I think the BPF runtime needs to make such actions defined, instead of
> doing a blind shift. It needs to check the size of the shift explicitly
> when handling the shift instruction.

Such ideas were brought up in the past and rejected.
We're not going to sacrifice performance to make behavior a bit more
'defined'. CPUs are doing it deterministically. It's the C standard
that needs fixing.

> Sure, but the point of UBSAN is to find and alert about undefined
> behavior, so we still need to fix this.

No. The undefined behavior of C standard doesn't need "fixing" most of the time.
_______________________________________________
Linux-kernel-mentees mailing list
Linux-kernel-mentees@lists.linuxfoundation.org
https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees

  reply	other threads:[~2021-06-10 17:54 UTC|newest]

Thread overview: 47+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-03-10 16:05 [syzbot] UBSAN: shift-out-of-bounds in ___bpf_prog_run syzbot
2021-03-28  3:38 ` syzbot
2021-06-02 21:27   ` [PATCH v3] bpf: core: fix " Kurt Manucredo
2021-06-02 21:27     ` Kurt Manucredo
2021-06-03  4:43     ` Greg KH
2021-06-03  4:43       ` Greg KH
2021-06-05 15:01       ` [PATCH v4] " Kurt Manucredo
2021-06-05 15:01         ` Kurt Manucredo
2021-06-05 17:55         ` Yonghong Song
2021-06-05 17:55           ` Yonghong Song via Linux-kernel-mentees
2021-06-05 19:10           ` Alexei Starovoitov
2021-06-05 19:10             ` Alexei Starovoitov
2021-06-05 21:39             ` Yonghong Song
2021-06-05 21:39               ` Yonghong Song via Linux-kernel-mentees
2021-06-06 19:44               ` Kurt Manucredo
2021-06-06 19:44                 ` Kurt Manucredo
2021-06-07  7:38             ` Dmitry Vyukov
2021-06-07  7:38               ` Dmitry Vyukov
2021-06-07  7:38               ` Dmitry Vyukov via Linux-kernel-mentees
2021-06-09 18:20               ` Kees Cook
2021-06-09 18:20                 ` Kees Cook
2021-06-09 23:40                 ` Yonghong Song
2021-06-09 23:40                   ` Yonghong Song via Linux-kernel-mentees
2021-06-10  5:32                   ` Dmitry Vyukov
2021-06-10  5:32                     ` Dmitry Vyukov
2021-06-10  5:32                     ` Dmitry Vyukov via Linux-kernel-mentees
2021-06-10  6:06                     ` Yonghong Song
2021-06-10  6:06                       ` Yonghong Song via Linux-kernel-mentees
2021-06-10 17:06                       ` Kees Cook
2021-06-10 17:06                         ` Kees Cook
2021-06-10 17:52                         ` Alexei Starovoitov [this message]
2021-06-10 17:52                           ` Alexei Starovoitov
2021-06-10 17:52                           ` Alexei Starovoitov
2021-06-10 20:00                           ` Eric Biggers
2021-06-10 20:00                             ` Eric Biggers
2021-06-15 16:42                             ` [PATCH v5] " Kurt Manucredo
2021-06-15 18:51                               ` Edward Cree
2021-06-15 19:33                                 ` Eric Biggers
2021-06-15 21:08                                   ` Daniel Borkmann
2021-06-15 21:32                                     ` Eric Biggers
2021-06-15 21:38                                       ` Eric Biggers
2021-06-15 21:54                                         ` Daniel Borkmann
2021-06-15 22:07                                           ` Eric Biggers
2021-06-15 22:31                                             ` Kurt Manucredo
2021-06-17 10:09                                             ` Daniel Borkmann
2021-06-06 19:15           ` [PATCH v4] " Kurt Manucredo
2021-06-06 19:15             ` Kurt Manucredo

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to='CAADnVQKMwKYgthoQV4RmGpZm9Hm-=wH3DoaNqs=UZRmJKefwGw@mail.gmail.com' \
    --to=alexei.starovoitov@gmail.com \
    --cc=andrii@kernel.org \
    --cc=ast@kernel.org \
    --cc=bpf@vger.kernel.org \
    --cc=clang-built-linux@googlegroups.com \
    --cc=daniel@iogearbox.net \
    --cc=davem@davemloft.net \
    --cc=dvyukov@google.com \
    --cc=fuzzybritches0@gmail.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=hawk@kernel.org \
    --cc=john.fastabend@gmail.com \
    --cc=kafai@fb.com \
    --cc=kasan-dev@googlegroups.com \
    --cc=keescook@chromium.org \
    --cc=kernel-hardening@lists.openwall.com \
    --cc=kpsingh@kernel.org \
    --cc=kuba@kernel.org \
    --cc=linux-kernel-mentees@lists.linuxfoundation.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=nathan@kernel.org \
    --cc=ndesaulniers@google.com \
    --cc=netdev@vger.kernel.org \
    --cc=skhan@linuxfoundation.org \
    --cc=songliubraving@fb.com \
    --cc=syzbot+bed360704c521841c85d@syzkaller.appspotmail.com \
    --cc=syzkaller-bugs@googlegroups.com \
    --cc=yhs@fb.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.