Re: [PATCH net-next 1/1] net: validate HWTSTAMP ioctl parameters

[Richard Cochran <richardcochran@gmail.com>]

On Wed, Oct 19, 2011 at 10:16:56PM +0100, Ben Hutchings wrote:
> On Fri, 2011-10-14 at 11:37 +0200, Richard Cochran wrote:
> > This patch adds a sanity check on the values provided by user space for
> > the hardware time stamping configuration. If the values lie outside of
> > the absolute limits, then the ioctl request will be denied.
> [...]
> 
> What does this validation buy us?  The driver still has to copy the
> values into kernel space again, at which point they may have been
> changed to be invalid.  Depending on how the driver uses them (perhaps
> as array indices), it may have to validate them again to avoid a
> security vulnerability.

Oops, you are right.

The drivers will really need to check the configuration again in any
case, since no driver will support every option.

I understood David's request as simply a sanity check on the absolute
limits.

> I think that either SIOCHWTSTAMP should be handled through a discrete
> device operation (not ndo_ioctl) which receives a pointer to the
> validated structure in kernel memory,

Okay, I'll take a stab at this.

> or a validation function should be
> exported to drivers so that they can call it from their ndo_ioctl
> implementations after copying the structure into kernel memory.

I think it better to do the sanity check in one place, to guard
against lazy or sloppy drivers.

Thanks,
Richard