On Fri, Jun 19, 2020 at 08:50:52PM +0000, Changming Liu wrote: > > > This patch series changes all appropriate instances of signed char > > > arrays or buffer to unsigned char. > > > > > > For each patch, if changing one variable to u8 involves its > > > callers/callees, then those changes are included in that patch as > > > well. > > > > > > This doesn't apply to ioctl functions, since the types for buffer of > > > ioctl-like functions needs to be char* instead of u8* to keep the > > > compiler happy. > > > > Why is that? What is forcing those types to be that way? These are all self- > > contained in the driver itself, so they should be able to be changed, right? > > > > Do you have an example of a function that you want to change but somehow > > can not? > > > Sorry for this confusion, I should have put more context into this patch. > This is a re-send of a former patch which was rejected by kernel build > test robot when I tried to change all char instances of this driver to > u8 in order to remove any potential undefined behaviors. > > This patch(also the former rejected one) were based on a former discussion > with you, the email was quite lengthy, so I attached the link here for > your reference. https://www.spinics.net/lists/linux-usb/msg196153.html > > In conclusion, only the one I noted in the link has security implication > and should be fixed, the other changes from char to u8 are just > "in case". > > If you still think it's needed to change all instances > of char in this driver to u8, I'll enrich the patch note(which I should > have done earlier) and re-send the patch series again. > Or if you think just fixing that specific UB in sisusb_write_mem_bulk > is enough, I'll submit another patch. I think cleaning up everything is good, so fixing that up and resending it would be great to have. thanks, greg k-h
