On Wed, Mar 24, 2021 at 01:52:46PM -0300, Jason Gunthorpe wrote: > On Wed, Mar 24, 2021 at 09:13:35AM -0700, Dan Williams wrote: > > > Which is just: > > > > device_initialize() > > dev_set_name() > > > > ...then the name is set as early as the device is ready to filled in > > with other details. Just checking for dev_set_name() failures does not > > move the api forward in my opinion. > > This doesn't work either as the release function must be set after > initialize but before dev_set_name(), otherwise we both can't and must > call put_device() after something like this fails. > > I can't see an option other than bite the bullet and fix things. > > A static tool to look for these special lifetime rules around the > driver core would be nice. If y'all are specific enough about what you want, then I can write the check for you. What I really want is some buggy sample code and the warning you want me to print. I kind of vaguely know that devm_ life time rules are tricky but I don't know the details. https://lore.kernel.org/dmaengine/CAPcyv4g2Odzusx621vatPbA041NXMmc1JK_3oSNM-EOPwDaxqA@xxxxxxxxxxxxxx/T/#m18d39df4097b12a9a5bdf51bb86b31cd0c6c0136 The error handling in idxd_setup_interrupts() and idxd_allocate_wqs/engines/groups() is faulty. + for (i = 0; i < idxd->max_engines; i++) { + engine = kzalloc_node(sizeof(*engine), GFP_KERNEL, dev_to_node(dev)); + if (!engine) { + rc = -ENOMEM; + goto err; Imagine that kzalloc_node() fails on the first iteration. + } + + idxd->engines[i] = engine; + } + + return 0; + + err: + while (--i) + kfree(idxd->engines[i]); The while loop should be while (i--) or while (--i >= 0). Otherwise, --i is -1 so this will loop downwards until it crashes. regards, dan carpenter
