> > That's the point. Your patch doesn't check if the length is valid. You > > rely on the caller to know no more than 32 bytes can be transferred. It > > shouldn't limit the subfunction to transfer more than 32 bytes if > > hardware can support it by multiple transactions. If not, print out an > > error message. > > > > It is customary for kernel functions to only validate parameters > wherever a value enters or leaves the kernel, or at least a logical > boundary. Anything else would blow up the kernel size to a point where > it would be unusable. The patch checks if the block length received from > the SMBus slave is correct, which is exactly what it is supposed to do. > > If you look closely, you may realize that the mpc_read also doesn't > validate of any of the other parameters. Are you going to request that > such validations be added as well ? How about the other functions in > this driver ? Should each function also validate each of its > parameters ? If not, where do you draw the line ? > > Besides, the caller is the SMBus block read function, which does know > that SMBus block reads are limited to 32 bytes (plus length). > I am going to let it go for now. Obviously your patch is working when length=1. Probably it will never be called with length other than 1 for SMBus block read. It would be nicer if you could put a comment there just in case in the future someone runs into a case where length+=byte causes a problem. York -- To unsubscribe from this list: send the line "unsubscribe linux-i2c" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
