On Mon 2019-10-28 14:09:29, Alexander Potapenko wrote:
> On Thu, Oct 24, 2019 at 2:46 PM Petr Mladek <pmladek@xxxxxxxx> wrote:
> > On Wed 2019-10-23 19:57:39, Alexander Potapenko wrote:
> > > What I'm seeing now is that e.g. in the following case:
> > > ptr = kmalloc(sizeof(int), GFP_KERNEL);
> > > if (ptr)
> > > pr_info("true\n");
> > > else
> > > pr_info("false\n");
> > >
> > > KMSAN detects errors within pr_info(), namely in vprintk_store().
> > > If I understand correctly, printing from that point causes printk to
> > > use the per-cpu buffer, which is flushed once we're done with the
> > > original printing:
> > >
> > > [ 58.984971][ T8390] BUG: KMSAN: uninit-value in
> > > kmsan_handle_vprintk+0xa0/0xf0
> > > ...
> > > [ 59.061976][ C0] BUG: KMSAN: uninit-value in vsnprintf+0x3276/0x32b0
> > > ...
> > > [ 59.062457][ C0] BUG: KMSAN: uninit-value in format_decode+0x17f/0x1900
> > > ...
> > > [ 59.062961][ C0] BUG: KMSAN: uninit-value in format_decode+0x17f/0x1900
> >
> > Please, do you have an explanation where the uninitialized values come
> > from? Is it a false positive? Or is there really a bug how the
> > printk() parameters are passed?
> I believe these are true bugs.
> The problem is, when we pass an uninitialized argument into printk(),
> KMSAN will report an error every time this uninitialized argument is
> used.
I see, thanks for explanation.
> E.g. for an uninitialized format string there'll be at least
> strlen(fmt) reports in format_decode(), followed by several reports in
> vsnprintf().
> Although these reports seem to be real, printing only the first of
> them should be more than enough.
Isn't this a generic problem? I mean that uninitialized values
can be passed and used also in many other locations.
printk() is special because this problem causes infinite loop. But it
would make sense to report also any other problematic value only once.
> In the future we'll actually want KMSAN to check every printk()
> argument (which will require parsing the format string to figure out
> the arguments' lengths), but disable reports within printk.
What is the motivation for this, please?
It looks to me that you want to do very paranoid checks of variables
passed to printk()? Do you want to prevent printk() crashes? Or
do you want to make sure that printk() produces correct values?
>From my POV, printk() is debugging tool. It is used to show values
that people are interested in. On one hand, it might make sense to warn
people that a particular value was not initalized. On the other hand,
printk() is not important for the kernel behavior. It just
reads values and does not affect any behavior.
I would like to understand how many printk-related code is
worth the effort.
Best Regards,
Petr
