On Thu, Jun 5, 2025 at 12:16 PM Pratyush Yadav <pratyush@xxxxxxxxxx> wrote:
>
> 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@xxxxxxxxxx>
> > ---
> [...]
> > +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)?
Thank you, David Matlack also noticed this leak, I fixed it.
>
> > + 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.
>
Done, thank you.
Pasha
