Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
MIME-Version: 1.0
References: <20190617123109.667090-1-arnd@arndb.de> <20190617140210.GB3436@hirez.programming.kicks-ass.net>
In-Reply-To: <20190617140210.GB3436@hirez.programming.kicks-ass.net>
From:   Arnd Bergmann <arnd@arndb.de>
Date:   Tue, 18 Jun 2019 14:56:07 +0200
Message-ID: <CAK8P3a3iwWOkMBL-H3h5aSaHKjKWFce22rvydvVE=3uMfeOhVg@mail.gmail.com>
Subject: Re: [PATCH] ubsan: mark ubsan_type_mismatch_common inline
To:     Peter Zijlstra <peterz@infradead.org>
Cc:     Andrey Ryabinin <aryabinin@virtuozzo.com>,
        Andrew Morton <akpm@linux-foundation.org>,
        Josh Poimboeuf <jpoimboe@redhat.com>,
        Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
        "# 3.4.x" <stable@vger.kernel.org>
Content-Type: text/plain; charset="UTF-8"
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On Mon, Jun 17, 2019 at 4:02 PM Peter Zijlstra <peterz@infradead.org> wrote:
>
> On Mon, Jun 17, 2019 at 02:31:09PM +0200, Arnd Bergmann wrote:
> > objtool points out a condition that it does not like:
> >
> > lib/ubsan.o: warning: objtool: __ubsan_handle_type_mismatch()+0x4a: call to stackleak_track_stack() with UACCESS enabled
> > lib/ubsan.o: warning: objtool: __ubsan_handle_type_mismatch_v1()+0x4a: call to stackleak_track_stack() with UACCESS enabled
> >
> > I guess this is related to the call ubsan_type_mismatch_common()
> > not being inline before it calls user_access_restore(), though
> > I don't fully understand why that is a problem.
>
> The rules are that when AC is set, one is not allowed to CALL schedule,
> because scheduling does not save/restore AC.  Preemption, through the
> exceptions is fine, because the exceptions do save/restore AC.
>
> And while most functions do not appear to call into schedule, function
> trace ensures that every single call does in fact call into schedule.
> Therefore any CALL (with AC set) is invalid.

I see that stackleak_track_stack is already marked 'notrace',
since we must ensure we don't recurse when calling into it from
any of the function trace logic.

Does that mean we could just mark it as another safe call?

--- a/tools/objtool/check.c
+++ b/tools/objtool/check.c
@@ -486,6 +486,7 @@ static const char *uaccess_safe_builtin[] = {
        "__ubsan_handle_type_mismatch",
        "__ubsan_handle_type_mismatch_v1",
        /* misc */
+       "stackleak_track_stack",
        "csum_partial_copy_generic",
        "__memcpy_mcsafe",
        "ftrace_likely_update", /* CONFIG_TRACE_BRANCH_PROFILING */


> Maybe we should disable stackleak when building ubsan instead? We
> already disable stack-protector when building ubsan.

I couldn't find out how that is done.

> > Fixes: 42440c1f9911 ("lib/ubsan: add type mismatch handler for new GCC/Clang")
>
> I don't think this is quite right, because back then there wasn't any
> uaccess validation.

Right.

       Arnd