On Sat, 2018-06-23 at 11:07 -0400, Alan Stern wrote: > On Sat, 23 Jun 2018, Benjamin Herrenschmidt wrote: > > > Hi folks ! > > > > I noticed some UDC drivers setup a "desc" pointer for ep0.desc for the > > driver, some don't. > > > > Is it officially needed ? > > No, it isn't. The only "core" routine that requires a desc pointer in > the endpoint structure is usb_ep_enable, and that routine explicitly > says it may not be called for ep0. > > > Additionally some UCDs NULL out the desc pointer in ep_disable, is that > > also a requirement ? > > No. A disabled endpoint doesn't have any requirements on its > descriptor or lack thereof. > > > I somewhat fear that if somebody calls stuff like usb_ep_align() on ep0 > > it will crash without a valid desc pointer... > > That would be a pretty strange thing to do. I'd say it's not worth > worrying about until somebody makes that mistake. Well, things like usb_endpoint_maxp() seem not *completely* far fetched but yeah EP0 is usually special enough that it should be ok. That said, I noticed some UDC drivers to setup a dummy desc there so I was wondering ... > > That leads me to wonder, should we sprinkle null checks (and maybe > > WARN_ON_ONCE) on some of those accessors to catch those cases ? > > I think segmentation faults will do a good job of catching them. :-) True, however in many practical cases, a WARN_ON and returning an error (or a semi-sane value) will go a long way in helping debug things, for example not crashing the machine and allowing a rmmod/insmod of a new driver rather than a reboot :-) Also not all embedded systems have an easily accessible serial console. Anyway, I'll dig a bit more see if I can find what exactly is going on, it's a report i have from somebody backporting to an old kernel so it will require extra digging. Cheers, Ben. -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
