Messages in this thread | | | Subject | Re: [PATCH] net: sched: Fix memory exposure from short TCA_U32_SEL | From | Joe Perches <> | Date | Sun, 26 Aug 2018 11:57:57 -0700 |
| |
On Sun, 2018-08-26 at 18:32 +0100, Al Viro wrote: > On Sat, Aug 25, 2018 at 11:19:30PM -0700, Kees Cook wrote: > > > > - n = kzalloc(sizeof(*n) + s->nkeys*sizeof(struct tc_u32_key), GFP_KERNEL); > > > > + n = kzalloc(offsetof(typeof(*n), sel) + sel_size, GFP_KERNEL); > > > > > > ITYM > > > n = kzalloc(offsetof(struct tc_u_common, sel.keys[s->nkeys]), GFP_KERNEL); > > > > I prefer to reuse sel_size and keep typeof() to keep things tied to > > "n" more directly. *shrug* > > This is rather search-hostile, though. Fresh example from the same > area: where are struct tcf_proto instances created? Is it true that > each is followed by ->ops->init()? Is it true that ->ops->init() > is never called twice for the same instance? Is it true that > ->ops->destroy() is called exactly once between successful ->ops->init() > and freeing the object? > > That's precisely the kind of questions you end up asking when learning > a new area. Your variant makes those harder to answer; it does make > it easier to catch local problems on casual grep, but it's hell both > on the newbies trying to make sense of an area and on the old hands > from different areas. > > That, BTW, is why I hate the use of sizeof(*p) in kmalloc, etc. > arguments. typeof is even worse in that respect.
True. Semantic searches via tools like coccinelle could help here but those searches are quite a bit slower than straightforward greps.
| |