On Thu, Jan 12, 2017 at 11:29:09AM +1100, Cyril Bur wrote:
> +static ssize_t lpc_ctrl_read(struct file *file, char __user *buf,
> + size_t count, loff_t *ppos)
> +{
> + if (!access_ok(VERIFY_WRITE, buf, count))
> + return -EFAULT;
> +
> + return -EPERM;
> +}
> +
> +static ssize_t lpc_ctrl_write(struct file *file, const char __user *buf,
> + size_t count, loff_t *ppos)
> +{
> + if (!access_ok(VERIFY_READ, buf, count))
> + return -EFAULT;
> +
> + return -EPERM;
> +}
Those functions don't actually do anything, so why even include them?
And don't call access_ok(), it's racy and no driver should ever do that.
> +static long lpc_ctrl_ioctl(struct file *file, unsigned int cmd,
> + unsigned long param)
> +{
> + long rc;
> + struct lpc_mapping map;
> + struct lpc_ctrl *lpc_ctrl = file_lpc_ctrl(file);
> + void __user *p = (void __user *)param;
> +
> + switch (cmd) {
> + case LPC_CTRL_IOCTL_SIZE:
> + return copy_to_user(p, &lpc_ctrl->size,
> + sizeof(lpc_ctrl->size)) ? -EFAULT : 0;
> + case LPC_CTRL_IOCTL_MAP:
> + if (copy_from_user(&map, p, sizeof(map)))
> + return -EFAULT;
> +
> +
> + /*
> + * The top half of HICR7 is the MSB of the BMC address of the
> + * mapping.
> + * The bottom half of HICR7 is the MSB of the HOST LPC
> + * firmware space address of the mapping.
> + *
> + * The 1 bits in the top of half of HICR8 represent the bits
> + * (in the requested address) that should be ignored and
> + * replaced with those from the top half of HICR7.
> + * The 1 bits in the bottom half of HICR8 represent the bits
> + * (in the requested address) that should be kept and pass
> + * into the BMC address space.
> + */
> +
> + rc = regmap_write(lpc_ctrl->regmap, HICR7,
> + (lpc_ctrl->base | (map.hostaddr >> 16)));
> + if (rc)
> + return rc;
> +
> + rc = regmap_write(lpc_ctrl->regmap, HICR8,
> + (~(map.size - 1)) | ((map.size >> 16) - 1));
Look Ma, a kernel exploit!
{sigh}
Your assignment is to go find a whiteboard/blackboard/whatever and write
on it 100 times:
All input is evil.
I want to see the picture of that before you send any more kernel patches.
> +static int lpc_ctrl_release(struct inode *inode, struct file *file)
> +{
> + atomic_dec(&lpc_ctrl_open_count);
Totally unneeded and unnecessary, why do you care?
Again, use UIO, it will save you from yourself...
thanks,
greg k-h
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
