Re: [RFC][2.5] generic_usercopy() function (resend, forgot the patches)

[Christoph Hellwig <hch@infradead.org>]

This function is small and very useful, I think it should be included unconditional
and the prototype maybe moved to kernel.h.

> +int
> +generic_usercopy(struct inode *inode, struct file *file,
> +		unsigned int cmd, unsigned long arg,
> +		int (*func)(struct inode *inode, struct file *file,
> +		unsigned int cmd, void *arg))

The name is a bit mislead.  maybe ioctl_usercopy?  ioctl_uaccess?
Also file/inode should go away from the prototype (and the callback).
Only file is needed because inode == file->f_dentry->d_inode, and even
that one should be just some void *data instead.



> +	char	sbuf[128];
> +	void    *mbuf = NULL;
> +	void	*parg = NULL;
> +	int	err  = -EINVAL;
> +
> +	/*  Copy arguments into temp kernel buffer  */
> +	switch (_IOC_DIR(cmd)) {
> +	case _IOC_NONE:
> +		parg = (void *)arg;
> +		break;
> +	case _IOC_READ: /* some v4l ioctls are marked wrong ... */

That's crap.  Please move this workaround to v4l not into generic code.

> +	case _IOC_WRITE:
> +	case (_IOC_WRITE | _IOC_READ):
> +		if (_IOC_SIZE(cmd) <= sizeof(sbuf)) {
> +			parg = sbuf;
> +		} else {
> +			/* too big to allocate from stack */
> +			mbuf = kmalloc(_IOC_SIZE(cmd),GFP_KERNEL);
> +			if (NULL == mbuf)
> +				return -ENOMEM;
> +			parg = mbuf;

I wonder whether you should just kmalloc always. 

> +	/* call driver */
> +	err = func(inode, file, cmd, parg);
> +	if (err == -ENOIOCTLCMD)
> +		err = -EINVAL;

I don't think this is the right place for this substitution - leave it to
the drivers.