Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Date:   Fri, 26 Feb 2021 18:33:09 -0500
From:   Steven Rostedt <rostedt@goodmis.org>
To:     Linus Torvalds <torvalds@linux-foundation.org>
Cc:     Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
        Ingo Molnar <mingo@kernel.org>,
        Andrew Morton <akpm@linux-foundation.org>,
        Masami Hiramatsu <mhiramat@kernel.org>,
        Jacob Wen <jian.w.wen@oracle.com>
Subject: Re: [PATCH 0/2] tracing: Detect unsafe dereferencing of pointers
 from trace events
Message-ID: <20210226183309.0edcc54d@gandalf.local.home>
In-Reply-To: <CAHk-=wiWF=ah_q1HBVUth2vuBx2TieN8U331y5FhXiehX-+=TQ@mail.gmail.com>
References: <20210226185909.100032746@goodmis.org>
        <CAHk-=wiWF=ah_q1HBVUth2vuBx2TieN8U331y5FhXiehX-+=TQ@mail.gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=US-ASCII
Content-Transfer-Encoding: 7bit
Precedence: bulk

On Fri, 26 Feb 2021 14:21:00 -0800
Linus Torvalds <torvalds@linux-foundation.org> wrote:

> > The second patch handles strings "%s" [..]  
> 
> Doing this at runtime really feels like the wrong thing to do.
> 
> It won't even protect us from what happened - people like me and
> Andrew won't even run those tracepoints in the first place, so we
> won't notice.
> 
> It really would be much better in every respect to have this done by
> checkpatch, I think.

They are not mutually exclusive. We could have both. One thing that's nice
about this patch is that it removes the possibility of a real bug. That is,
it will catch the dereferencing of a string that is not valid, WARN about
it, but it wont try to dereference it (outside of the
strcpy_from_kernel_nofault()). And hopefully the warning and lack of data
they want, will have this get caught during development.

Also, there's cases that %s is allowed to reference data that I don't know
if checkpatch would be able to differentiate.

As for the other pointer dereferences (the first patch), those get caught
at boot up if they are compiled in. That is, you don't need to have the
events enabled. The boot up code will do the verification on all events
that are loaded (allyesconfig will catch all of them, which I need to try
to boot with this code).

-- Steve