RE: [PATCH v3 2/4] bus: fsl-mc: add fsl-mc userspace support

[Ioana Ciornei <ioana.ciornei@nxp.com>]

> > +static int fsl_mc_uapi_send_command(unsigned long arg,
> > +				    struct fsl_mc_io *mc_io)
> > +{
> > +	struct fsl_mc_command mc_cmd;
> > +	int error;
> > +
> > +	error = copy_from_user(&mc_cmd, (void __user *)arg, sizeof(mc_cmd));
> > +	if (error)
> > +		return -EFAULT;
> > +
> > +	error = mc_send_command(mc_io, &mc_cmd);
> > +	if (error)
> > +		return error;
> > +
> > +	error = copy_to_user((void __user *)arg, &mc_cmd, sizeof(mc_cmd));
> > +	if (error)
> > +		return -EFAULT;
> > +
> > +	return 0;
> > +}
> 
> I know you said that "the firmware will properly verify the command"
> already, but given that I used to be a firmware developer a long time ago, I can
> almost guarantee that this will cause problems in the future.
> 
> Want to make a friendly bet about this?
> 
> What is the odds that your firmware api/interface has been properly fuzzed such
> that all possible combinations of bad commands will really not do horrible things
> to the hardware/system?
> 
> Are you all willing to bet the system intregrity on this?  If so, ok, it's your systems
> :)
> 
> Personally, I think you need to add a "known whitelist" and do some sort of
> sanity checking here.
> 

I can add a whitelist on the command ids that can be received from userspace but
leave the parameter parsing to the firmware to process and interpret depending on the
current system settings.

Would that be a viable option from your point of view?

Ioana C

> thanks,
> 
> greg k-h