Re: [PATCH v4.19-rc7] treewide: Replace more open-coded allocation size multiplications

From: Joel Fernandes
Date: Fri Oct 05 2018 - 23:14:42 EST


On Fri, Oct 05, 2018 at 05:22:35PM -0700, Greg KH wrote:
> On Fri, Oct 05, 2018 at 05:04:16PM -0700, Kees Cook wrote:
> > On Fri, Oct 5, 2018 at 4:51 PM, Greg KH <gregkh@xxxxxxxxxxxxxxxxxxx> wrote:
> > > On Fri, Oct 05, 2018 at 04:35:59PM -0700, Kees Cook wrote:
> > >> As done treewide earlier, this catches several more open-coded
> > >> allocation size calculations that were added to the kernel during the
> > >> merge window. This performs the following mechanical transformations
> > >> using Coccinelle:
> > >>
> > >> kvmalloc(a * b, ...) -> kvmalloc_array(a, b, ...)
> > >> kvzalloc(a * b, ...) -> kvcalloc(a, b, ...)
> > >> devm_kzalloc(..., a * b, ...) -> devm_kcalloc(..., a, b, ...)
> > >>
> > >> Signed-off-by: Kees Cook <keescook@xxxxxxxxxxxx>
> > >
> > > Has this had any testing in linux-next?
> >
> > No; they're mechanical transformations (though I did build test them).
> > If you want I could add this to linux-next for a week?
>
> That would be good, thanks.
>
> > > And when was "earlier"?
> >
> > v4.18, when all of these were originally eliminated:
> >
> > 026f05079b00 treewide: Use array_size() in f2fs_kzalloc()
> > c86065938aab treewide: Use array_size() in f2fs_kmalloc()
> > 76e43e37a407 treewide: Use array_size() in sock_kmalloc()
> > 84ca176bf54a treewide: Use array_size() in kvzalloc_node()
> > fd7becedb1f0 treewide: Use array_size() in vzalloc_node()
> > fad953ce0b22 treewide: Use array_size() in vzalloc()
> > 42bc47b35320 treewide: Use array_size() in vmalloc()
> > a86854d0c599 treewide: devm_kzalloc() -> devm_kcalloc()
> > 3c4211ba8ad8 treewide: devm_kmalloc() -> devm_kmalloc_array()
> > 778e1cdd81bb treewide: kvzalloc() -> kvcalloc()
> > 344476e16acb treewide: kvmalloc() -> kvmalloc_array()
> > 590b5b7d8671 treewide: kzalloc_node() -> kcalloc_node()
> > 6396bb221514 treewide: kzalloc() -> kcalloc()
> > 6da2ec56059c treewide: kmalloc() -> kmalloc_array()
> >
> > The new patch is catching new open-coded multiplications introduced in v4.19.
>
> As this is getting smaller, why not just break it up and do it through
> all of the different subsystems instead of one large patch?
>
> And do we have a way to add a rule to 0-day to catch these so that they
> get a warning when they are added again?

They could just be added to scripts/coccinelle and 0-day will report them?

For example, 0-day ran scripts/coccinelle/api/platform_no_drv_owner.cocci on
a recently submitted patch and reported it here:
https://lore.kernel.org/lkml/201808301856.vMNJerSs%25fengguang.wu@xxxxxxxxx/

But I'm not sure if 0-day runs make coccicheck on specific semantic patches,
or runs all of them (CC'd Fengguang).

thanks,

- Joel