Hi Dan, On Fri, Nov 25, 2016 at 10:20:24PM +0300, Dan Carpenter wrote: > On Fri, Nov 25, 2016 at 06:02:45PM +0200, Laurent Pinchart wrote: > > Sakari Ailus (CC'ed) has expressed the opinion that we might want to go one > > step further and treat error pointers the same way we treat NULL or ZERO > > pointers today, by just returning without logging anything. The reasoning is > > that accepting a NULL pointer in kfree() was decided before we made extensive > > use of allocation APIs returning error pointers, so it could be time to update > > kfree() based on the current allocation usage patterns. > > Just don't free things that haven't been allocated. That honestly seems > like a simple rule to me, whenever I touch error handling code it feels > better and simpler after I fix the bugs. Error handling doesn't have to > be complicated if you just follow the rules. kfree() explicitly allows passing a NULL pointer to it; drivers often call kfree() on objects possibly allocated using kmalloc() and friends. This makes error handling easier in drivers which in turn decreases the probability of bugs, the other side of which we've already seen in form of the bug this patch fixes. Previously interfaces that allocated memory tended to either allocate that memory or in failing to do so, returned error in form of a NULL pointer. memdup_user() breaks that assumption by returning a negative error value as a pointer instead. I suppose one of the motivations of memdup_user() has been to reduce complexity of driver code as well as framework code dealing with implementing IOCTLs but at least in this case the end result was an introduction of a bug. This would not have happened in the first place if the API of functions dealing with releasing memory had been updated as well. -- Kind regards, Sakari Ailus e-mail: sakari.ailus@xxxxxx XMPP: sailus@xxxxxxxxxxxxxx -- To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
