Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
MIME-Version: 1.0
References: <20220714205646.4023495-1-ndesaulniers@google.com> <CAHk-=whnndsMXO=32vFn9-Osw4r8-NvROV1rtsbxjw9Uvj1WPA@mail.gmail.com>
In-Reply-To: <CAHk-=whnndsMXO=32vFn9-Osw4r8-NvROV1rtsbxjw9Uvj1WPA@mail.gmail.com>
From:   Nick Desaulniers <ndesaulniers@google.com>
Date:   Thu, 14 Jul 2022 14:38:28 -0700
Message-ID: <CAKwvOd=PB9Kpbnf3y0e5u6LrZ=Lv2Ex8O7y=VY63D67Jf2eyHQ@mail.gmail.com>
Subject: Re: [PATCH] ubsan: disable UBSAN_DIV_ZERO for clang
To:     Linus Torvalds <torvalds@linux-foundation.org>
Cc:     Kees Cook <keescook@chromium.org>,
        Sudip Mukherjee <sudipm.mukherjee@gmail.com>,
        Nathan Chancellor <nathan@kernel.org>,
        Tom Rix <trix@redhat.com>, Marco Elver <elver@google.com>,
        Andrew Morton <akpm@linux-foundation.org>,
        Josh Poimboeuf <jpoimboe@kernel.org>,
        "Peter Zijlstra (Intel)" <peterz@infradead.org>,
        Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
        clang-built-linux <llvm@lists.linux.dev>,
        Alexander Potapenko <glider@google.com>
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

On Thu, Jul 14, 2022 at 2:25 PM Linus Torvalds
<torvalds@linux-foundation.org> wrote:
>
> The way you do that is by warning, and giving it defined behavior. It
> really is that simple.

int do_div (int dividend, int divisor) {
  return dividend / divisor;
}

has UB should divisor ever be zero, not much different from:

int deref (int *foo) {
  return *foo;
}

when foo is NULL.  Should the two of those be:

int do_div (int dividend, int divisor) {
  if (!divisor)
    return -EOOPS;
  return dividend / divisor;
}
int deref (int *foo) {
  if (!foo)
    return -EOOPS;
  return *foo;
}

or keep the unchecked versions and wait for a report from a user or
bot with a sanitizer splat?

I get the sanitizer doesn't work as advertised. I _agree_ with you.
Hence this patch (which I _think_ works towards your point, shouldn't
you Ack it?).  I feel like you're talking past me without addressing
my point, let me try rephrasing it:

I _additionally_ think we should be adding more checks to guard
against division by zero to the kernel sources.  Or are we happy to
wait and find out if divisors are ever zero and fix them as they pop
up/become problematic?
-- 
Thanks,
~Nick Desaulniers