Re: [PATCH 0/4] USB: sisusbvga: series of changes char to u8

[Greg KH <gregkh@linuxfoundation.org>]

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