MIME-Version: 1.0
References: <20221101222520.never.109-kees@kernel.org> <20221101223321.1326815-5-keescook@chromium.org>
 <CAMSo37W3gRkP02tSCxGX71ZDAt3WgPZrkTRTM6J1iQ4gvUS9vg@mail.gmail.com>
 <CANDhNCogJrvt=yEXFK-xVmGjkcRxSNGZUqUeNw2MV9bFRrwPdQ@mail.gmail.com> <63dbf04a.630a0220.2608a.0149@mx.google.com>
In-Reply-To: <63dbf04a.630a0220.2608a.0149@mx.google.com>
From:   John Stultz <jstultz@google.com>
Date:   Thu, 2 Feb 2023 10:56:29 -0800
Message-ID: <CANDhNCoJJs_jSAdr6uved=cK=-6+nzSJq3e_E3HgtiXHZXkVww@mail.gmail.com>
Subject: Re: [PATCH 5/6] driver core: Add __alloc_size hint to devm allocators
To:     Kees Cook <keescook@chromium.org>
Cc:     Yongqin Liu <yongqin.liu@linaro.org>,
        Vlastimil Babka <vbabka@suse.cz>,
        Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
        Rasmus Villemoes <rasmus.villemoes@prevas.dk>,
        Thomas Gleixner <tglx@linutronix.de>,
        Jason Gunthorpe <jgg@ziepe.ca>, Nishanth Menon <nm@ti.com>,
        Michael Kelley <mikelley@microsoft.com>,
        Dan Williams <dan.j.williams@intel.com>,
        Won Chung <wonchung@google.com>,
        David Gow <davidgow@google.com>,
        Christoph Lameter <cl@linux.com>,
        Pekka Enberg <penberg@kernel.org>,
        David Rientjes <rientjes@google.com>,
        Joonsoo Kim <iamjoonsoo.kim@lge.com>,
        Roman Gushchin <roman.gushchin@linux.dev>,
        Hyeonggon Yoo <42.hyeyoo@gmail.com>,
        Guenter Roeck <linux@roeck-us.net>,
        Andy Shevchenko <andriy.shevchenko@intel.com>,
        Paolo Abeni <pabeni@redhat.com>,
        Geert Uytterhoeven <geert@linux-m68k.org>,
        Nathan Chancellor <nathan@kernel.org>,
        Nick Desaulniers <ndesaulniers@google.com>,
        Tom Rix <trix@redhat.com>, linux-kernel@vger.kernel.org,
        linux-mm@kvack.org, linux-hardening@vger.kernel.org,
        llvm@lists.linux.dev, Sumit Semwal <sumit.semwal@linaro.org>
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

On Thu, Feb 2, 2023 at 9:18 AM Kees Cook <keescook@chromium.org> wrote:
> On Wed, Feb 01, 2023 at 12:11:41AM -0800, John Stultz wrote:
> > On Tue, Jan 31, 2023 at 11:36 PM Yongqin Liu <yongqin.liu@linaro.org> wrote:
> > > This change causes "Kernel panic - not syncing: BRK handler: Fatal exception"
> > > for the android-mainline based hikey960 build, with this commit reverted,
> > > there is no problem for the build to boot to the homescreen.
> > > Not sure if you have any idea about it and give some suggestions.
> > >
> > > Here is part of the kernel panic log:
...
> > Here as nr_sensors=1, we allocate only one structure for the array.
> > But then below that, we modify two entries, writing past the valid
> > array, and corrupting data when writing the second sensor values.
> >
> > data->sensor[0].id = HI3660_BIG_SENSOR;
> > data->sensor[0].irq_name = "tsensor_a73";
> > data->sensor[0].data = data;
> >
> > data->sensor[1].id = HI3660_LITTLE_SENSOR;
> > data->sensor[1].irq_name = "tsensor_a53";
> > data->sensor[1].data = data;
> >
> > I suspect nr_sensors needs to be set to 2.
> >
> > Nice work, Kees!
>
> Yay for compilers! :)

Well, I know it's not trivial to make the compilers catch these
things, so yay for you and others putting in all the effort on this as
well.

That said, making sense of the error message isn't completely trivial
either. I've been seeing a few cases recently of some of the new
compiler tooling (I pinged you earlier on a CFI one) causing errors
that developers aren't really sure how to address.  I know sometimes
it's not easy to surface the errors with context to what was wrong,
but at the risk of intense bike shedding, is there some way to provide
something like "Likely array bounds error" instead of just "BRK
handler: Fatal exception"?

> Was a patch sent to fix this driver?

I think YongQin is looking into it (either setting the nr_sensors
value to 2 or dropping the second sensor access).

thanks
-john