Re: [RFC v2 10/16] luo: luo_ioctl: add ioctl interface

[Pratyush Yadav <pratyush@kernel.org>]

On Thu, May 15 2025, Pasha Tatashin wrote:

> Introduce the user-space interface for the Live Update Orchestrator
> via ioctl commands, enabling external control over the live update
> process and management of preserved resources.
>
> Create a misc character device at /dev/liveupdate. Access
> to this device requires the CAP_SYS_ADMIN capability.
>
> A new UAPI header, <uapi/linux/liveupdate.h>, defines the necessary
> structures. The magic number is registered in
> Documentation/userspace-api/ioctl/ioctl-number.rst.
>
> Signed-off-by: Pasha Tatashin <pasha.tatashin@soleen.com>
> ---
[...]
> +static int luo_ioctl_fd_preserve(struct liveupdate_fd *luo_fd)
> +{
> +	struct file *file;
> +	int ret;
> +
> +	file = fget(luo_fd->fd);
> +	if (!file) {
> +		pr_err("Bad file descriptor\n");
> +		return -EBADF;
> +	}
> +
> +	ret = luo_register_file(&luo_fd->token, file);
> +	if (ret)
> +		fput(file);
> +
> +	return ret;
> +}
> +
> +static int luo_ioctl_fd_unpreserve(u64 token)
> +{

This leaks the refcount on the file that preserve took. Perhaps
luo_unregister_file() should return the file it unregistered, so this
can do fput(file)?

> +	return luo_unregister_file(token);
> +}
> +
> +static int luo_ioctl_fd_restore(struct liveupdate_fd *luo_fd)
> +{
> +	struct file *file;
> +	int ret;
> +	int fd;
> +
> +	fd = get_unused_fd_flags(O_CLOEXEC);
> +	if (fd < 0) {
> +		pr_err("Failed to allocate new fd: %d\n", fd);
> +		return fd;
> +	}
> +
> +	ret = luo_retrieve_file(luo_fd->token, &file);
> +	if (ret < 0) {
> +		put_unused_fd(fd);
> +
> +		return ret;
> +	}
> +
> +	fd_install(fd, file);
> +	luo_fd->fd = fd;
> +
> +	return 0;
> +}
> +
> +static int luo_open(struct inode *inodep, struct file *filep)
> +{
> +	if (!capable(CAP_SYS_ADMIN))
> +		return -EACCES;
> +
> +	if (filep->f_flags & O_EXCL)
> +		return -EINVAL;
> +
> +	return 0;
> +}
> +
> +static long luo_ioctl(struct file *filep, unsigned int cmd, unsigned long arg)
> +{
> +	void __user *argp = (void __user *)arg;
> +	struct liveupdate_fd luo_fd;
> +	enum liveupdate_state state;
> +	int ret = 0;
> +	u64 token;
> +
> +	if (_IOC_TYPE(cmd) != LIVEUPDATE_IOCTL_TYPE)
> +		return -ENOTTY;
> +
> +	switch (cmd) {
> +	case LIVEUPDATE_IOCTL_GET_STATE:
> +		state = READ_ONCE(luo_state);
> +		if (copy_to_user(argp, &state, sizeof(luo_state)))
> +			ret = -EFAULT;
> +		break;
> +
> +	case LIVEUPDATE_IOCTL_EVENT_PREPARE:
> +		ret = luo_prepare();
> +		break;
> +
> +	case LIVEUPDATE_IOCTL_EVENT_FREEZE:
> +		ret = luo_freeze();
> +		break;
> +
> +	case LIVEUPDATE_IOCTL_EVENT_FINISH:
> +		ret = luo_finish();
> +		break;
> +
> +	case LIVEUPDATE_IOCTL_EVENT_CANCEL:
> +		ret = luo_cancel();
> +		break;
> +
> +	case LIVEUPDATE_IOCTL_FD_PRESERVE:
> +		if (copy_from_user(&luo_fd, argp, sizeof(luo_fd))) {
> +			ret = -EFAULT;
> +			break;
> +		}
> +
> +		ret = luo_ioctl_fd_preserve(&luo_fd);
> +		if (!ret && copy_to_user(argp, &luo_fd, sizeof(luo_fd)))
> +			ret = -EFAULT;

luo_unregister_file() is needed here on error.

> +		break;
> +
> +	case LIVEUPDATE_IOCTL_FD_UNPRESERVE:
> +		if (copy_from_user(&token, argp, sizeof(u64))) {
> +			ret = -EFAULT;
> +			break;
> +		}
> +
> +		ret = luo_ioctl_fd_unpreserve(token);
> +		break;
> +
> +	case LIVEUPDATE_IOCTL_FD_RESTORE:
> +		if (copy_from_user(&luo_fd, argp, sizeof(luo_fd))) {
> +			ret = -EFAULT;
> +			break;
> +		}
> +
> +		ret = luo_ioctl_fd_restore(&luo_fd);
> +		if (!ret && copy_to_user(argp, &luo_fd, sizeof(luo_fd)))
> +			ret = -EFAULT;
> +		break;
> +
> +	default:
> +		pr_warn("ioctl: unknown command nr: 0x%x\n", _IOC_NR(cmd));
> +		ret = -ENOTTY;
> +		break;
> +	}
> +
> +	return ret;
> +}
[...]

-- 
Regards,
Pratyush Yadav