[prev in list] [next in list] [prev in thread] [next in thread] 

List:       target-devel
Subject:    Re: Bug report: KFIFO kfifo_init() may introduce buffer overflow
From:       Will Deacon <will () kernel ! org>
Date:       2019-08-29 21:51:32
Message-ID: 20190829215131.GA2404 () brain-police
[Download RAW message or body]

On Thu, Aug 29, 2019 at 01:00:21PM -0700, Linus Torvalds wrote:
> [ at the dentist, sorry for mobile html gunk ]
> 
> On Thu, Aug 29, 2019, 12:42 Kees Cook <keescook@chromium.org> wrote:
> 
>     On Thu, 
> 
>     I don't know this code at all, but note below...
> 
>     > +     __kfifo->mask = __KFIFO_MASK_SIZE(fifo); \
> 
>     I think this should be:
> 
>     +       __kfifo->mask = __KFIFO_MASK_SIZE(*__tmp); \
> 
>     ? 
> 
> 
> If it matters, we're in deep doo-doo. It only uses the type of the thing, not
> the value, so it's immaterial. I think it's easier to use the original type
> rather than the temporary that we created using it..
> 
> But that's the least of my worries in that code.

If it makes you feel any better [perhaps as a distraction from the dreaded
dentist], the memory ordering side of things is suspicious as well:

https://lore.kernel.org/lkml/CAGXu5jKkqf-9ksvNTCS5xgB_JtfvCc=Eot2uWYYP8rpoKLw=mg@mail.gmail.com/

When I started pulling on it, it all fell apart, so I haven't had a
chance to do a proper set of fixes.

Will
[prev in list] [next in list] [prev in thread] [next in thread]