Re: [PATCH 4/7] cdrom: factor out a cdrom_multisession helper
From: Damien Le Moal
Date: Thu Apr 23 2020 - 03:41:52 EST
On 2020/04/23 16:15, Christoph Hellwig wrote:
> Factor out a version of the CDROMMULTISESSION: ioctl handler that can
> be called directly from kernel space.
>
> Signed-off-by: Christoph Hellwig <hch@xxxxxx>
> ---
> drivers/cdrom/cdrom.c | 41 +++++++++++++++++++++++++----------------
> include/linux/cdrom.h | 2 ++
> 2 files changed, 27 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/cdrom/cdrom.c b/drivers/cdrom/cdrom.c
> index c91d1e138214..06896c07b133 100644
> --- a/drivers/cdrom/cdrom.c
> +++ b/drivers/cdrom/cdrom.c
> @@ -2295,37 +2295,46 @@ static int cdrom_read_cdda(struct cdrom_device_info *cdi, __u8 __user *ubuf,
> return cdrom_read_cdda_old(cdi, ubuf, lba, nframes);
> }
>
> -static int cdrom_ioctl_multisession(struct cdrom_device_info *cdi,
> - void __user *argp)
> +int cdrom_multisession(struct cdrom_device_info *cdi,
> + struct cdrom_multisession *info)
> {
> - struct cdrom_multisession ms_info;
> u8 requested_format;
> int ret;
>
> - cd_dbg(CD_DO_IOCTL, "entering CDROMMULTISESSION\n");
> -
> if (!(cdi->ops->capability & CDC_MULTI_SESSION))
> return -ENOSYS;
>
> - if (copy_from_user(&ms_info, argp, sizeof(ms_info)))
> - return -EFAULT;
> -
> - requested_format = ms_info.addr_format;
> + requested_format = info->addr_format;
> if (requested_format != CDROM_MSF && requested_format != CDROM_LBA)
> return -EINVAL;
> - ms_info.addr_format = CDROM_LBA;
> + info->addr_format = CDROM_LBA;
>
> - ret = cdi->ops->get_last_session(cdi, &ms_info);
> - if (ret)
> - return ret;
> + ret = cdi->ops->get_last_session(cdi, info);
> + if (!ret)
> + sanitize_format(&info->addr, &info->addr_format,
> + requested_format);
> + return ret;
> +}
> +EXPORT_SYMBOL_GPL(cdrom_multisession);
>
> - sanitize_format(&ms_info.addr, &ms_info.addr_format, requested_format);
> +static int cdrom_ioctl_multisession(struct cdrom_device_info *cdi,
> + void __user *argp)
> +{
> + struct cdrom_multisession info;
> + int ret;
> +
> + cd_dbg(CD_DO_IOCTL, "entering CDROMMULTISESSION\n");
>
> - if (copy_to_user(argp, &ms_info, sizeof(ms_info)))
> + if (copy_from_user(&info, argp, sizeof(info)))
> + return -EFAULT;
> + ret = cdrom_multisession(cdi, &info);
> + if (ret)
> + return ret;
> + if (copy_to_user(argp, &info, sizeof(info)))
> return -EFAULT;
>
> cd_dbg(CD_DO_IOCTL, "CDROMMULTISESSION successful\n");
> - return 0;
> + return ret;
> }
>
> static int cdrom_ioctl_eject(struct cdrom_device_info *cdi)
> diff --git a/include/linux/cdrom.h b/include/linux/cdrom.h
> index 008c4d79fa33..8543fa59da72 100644
> --- a/include/linux/cdrom.h
> +++ b/include/linux/cdrom.h
> @@ -94,6 +94,8 @@ struct cdrom_device_ops {
> struct packet_command *);
> };
>
> +int cdrom_multisession(struct cdrom_device_info *cdi,
> + struct cdrom_multisession *info);
> int cdrom_read_tocentry(struct cdrom_device_info *cdi,
> struct cdrom_tocentry *entry);
>
>
Looks OK to me.
Reviewed-by: Damien Le Moal <damien.lemoal@xxxxxxx>
--
Damien Le Moal
Western Digital Research