All of lore.kernel.org
 help / color / mirror / Atom feed
From: Kees Cook <kees@kernel.org>
To: Marco Elver <elver@google.com>
Cc: Nathan Chancellor <nathan@kernel.org>,
	Dmitry Vyukov <dvyukov@google.com>,
	kasan-dev@googlegroups.com, linux-kernel@vger.kernel.org,
	linux-hardening@vger.kernel.org
Subject: Re: [PATCH] kcsan: test: Adjust "expect" allocation type for kmalloc_obj
Date: Tue, 24 Feb 2026 15:28:03 -0800	[thread overview]
Message-ID: <202602241526.AE3F2F4A32@keescook> (raw)
In-Reply-To: <CANpmjNNZ-U4hT8LaW=V+q+NRPHb=fsxai86CBb1VdV8Pyo_xNA@mail.gmail.com>

On Tue, Feb 24, 2026 at 11:39:45PM +0100, Marco Elver wrote:
> On Tue, 24 Feb 2026 at 22:48, Kees Cook <kees@kernel.org> wrote:
> >
> > On Tue, Feb 24, 2026 at 11:09:44AM +0100, Marco Elver wrote:
> > > On Mon, 23 Feb 2026 at 23:22, Kees Cook <kees@kernel.org> wrote:
> > > >
> > > > Instead of depending on the implicit case between a pointer to pointers
> > > > and pointer to arrays, use the assigned variable type for the allocation
> > > > type so they correctly match. Solves the following build error:
> > > >
> > > > ../kernel/kcsan/kcsan_test.c: In function '__report_matches':
> > > > ../kernel/kcsan/kcsan_test.c:171:16: error: assignment to 'char (*)[512]' from incompatible pointer type 'char (*)[3][512]'
> > > > [-Wincompatible-pointer-types]
> > > >   171 |         expect = kmalloc_obj(observed.lines);
> > > >       |                ^
> > > >
> > > > Tested with:
> > > >
> > > > $ ./tools/testing/kunit/kunit.py run \
> > > >         --kconfig_add CONFIG_DEBUG_KERNEL=y \
> > > >         --kconfig_add CONFIG_KCSAN=y \
> > > >         --kconfig_add CONFIG_KCSAN_KUNIT_TEST=y \
> > > >         --arch=x86_64 --qemu_args '-smp 2' kcsan
> > > >
> > > > Reported-by: Nathan Chancellor <nathan@kernel.org>
> > > > Fixes: 69050f8d6d07 ("treewide: Replace kmalloc with kmalloc_obj for non-scalar types")
> > > > Signed-off-by: Kees Cook <kees@kernel.org>
> > > > ---
> > > > Cc: Marco Elver <elver@google.com>
> > > > Cc: Dmitry Vyukov <dvyukov@google.com>
> > > > Cc: <kasan-dev@googlegroups.com>
> > > > ---
> > > >  kernel/kcsan/kcsan_test.c | 2 +-
> > > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > >
> > > > diff --git a/kernel/kcsan/kcsan_test.c b/kernel/kcsan/kcsan_test.c
> > > > index 79e655ea4ca1..056fa859ad9a 100644
> > > > --- a/kernel/kcsan/kcsan_test.c
> > > > +++ b/kernel/kcsan/kcsan_test.c
> > > > @@ -168,7 +168,7 @@ static bool __report_matches(const struct expect_report *r)
> > > >         if (!report_available())
> > > >                 return false;
> > > >
> > > > -       expect = kmalloc_obj(observed.lines);
> > > > +       expect = kmalloc_obj(*expect);
> > >
> > > This is wrong. Instead of allocating 3x512 bytes it's now only
> > > allocating 512 bytes, so we get OOB below with this change. 'expect'
> > > is a pointer to a 3-dimensional array of 512-char arrays (matching
> > > observed.lines).
> >
> > Why did running the kunit test not trip over this? :(
> >
> > Hmpf, getting arrays allocated without an explicit cast seems to be
> > impossible. How about this:
> >
> >
> > diff --git a/kernel/kcsan/kcsan_test.c b/kernel/kcsan/kcsan_test.c
> > index 056fa859ad9a..ae758150ccb9 100644
> > --- a/kernel/kcsan/kcsan_test.c
> > +++ b/kernel/kcsan/kcsan_test.c
> > @@ -168,7 +168,7 @@ static bool __report_matches(const struct expect_report *r)
> >         if (!report_available())
> >                 return false;
> >
> > -       expect = kmalloc_obj(*expect);
> > +       expect = (typeof(expect))kmalloc_obj(observed.lines);
> 
> That works - or why not revert it back to normal kmalloc? There's
> marginal benefit for kmalloc_obj() in this case, and this really is
> just a bunch of char buffers - not a complex object. If there's still
> a benefit to be had from kmalloc_obj() here, I'm fine with the typeof
> cast.

Honestly... it's because I can't figure out how to make a exclusion for
this (nor how to get multidimensional array types) in Coccinelle to
avoid this case. (So re-running the conversion script will keep trying
to change this case.) And it's the only place in the kernel doing this
kind of thing. :P

I've sent v2 with the cast and a better commit log describing what's
happening.

-- 
Kees Cook

  reply	other threads:[~2026-02-24 23:28 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-02-23 22:22 [PATCH] kcsan: test: Adjust "expect" allocation type for kmalloc_obj Kees Cook
2026-02-24 10:09 ` Marco Elver
2026-02-24 21:48   ` Kees Cook
2026-02-24 22:39     ` Marco Elver
2026-02-24 23:28       ` Kees Cook [this message]
2026-02-24 22:41     ` Kees Cook
2026-02-24 22:43       ` Marco Elver

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=202602241526.AE3F2F4A32@keescook \
    --to=kees@kernel.org \
    --cc=dvyukov@google.com \
    --cc=elver@google.com \
    --cc=kasan-dev@googlegroups.com \
    --cc=linux-hardening@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=nathan@kernel.org \
    /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.