linux-kernel - Re: [PATCH 5/7] hfsplus: stop using ioctl_by

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Thu, 23 Apr 2020 07:42:26 +0000
From:   Damien Le Moal <Damien.LeMoal@....com>
To:     Christoph Hellwig <hch@....de>, Jens Axboe <axboe@...nel.dk>
CC:     Tim Waugh <tim@...erelk.net>, Borislav Petkov <bp@...en8.de>,
        Jan Kara <jack@...e.com>,
        "linux-block@...r.kernel.org" <linux-block@...r.kernel.org>,
        "linux-ide@...r.kernel.org" <linux-ide@...r.kernel.org>,
        "linux-scsi@...r.kernel.org" <linux-scsi@...r.kernel.org>,
        "linux-fsdevel@...r.kernel.org" <linux-fsdevel@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 5/7] hfsplus: stop using ioctl_by_bdev

On 2020/04/23 16:16, Christoph Hellwig wrote:
> Instead just call the CD-ROM layer functionality directly.
> 
> Signed-off-by: Christoph Hellwig <hch@....de>
> ---
>  fs/hfsplus/wrapper.c | 33 ++++++++++++++++++---------------
>  1 file changed, 18 insertions(+), 15 deletions(-)
> 
> diff --git a/fs/hfsplus/wrapper.c b/fs/hfsplus/wrapper.c
> index 08c1580bdf7a..61eec628805d 100644
> --- a/fs/hfsplus/wrapper.c
> +++ b/fs/hfsplus/wrapper.c
> @@ -127,31 +127,34 @@ static int hfsplus_read_mdb(void *bufptr, struct hfsplus_wd *wd)
>  static int hfsplus_get_last_session(struct super_block *sb,
>  				    sector_t *start, sector_t *size)
>  {
> -	struct cdrom_multisession ms_info;
> -	struct cdrom_tocentry te;
> -	int res;
> +	struct cdrom_device_info *cdi = disk_to_cdi(sb->s_bdev->bd_disk);
>  
>  	/* default values */
>  	*start = 0;
>  	*size = i_size_read(sb->s_bdev->bd_inode) >> 9;
>  
>  	if (HFSPLUS_SB(sb)->session >= 0) {
> +		struct cdrom_tocentry te;
> +
> +		if (!cdi)
> +			return -EINVAL;
> +
>  		te.cdte_track = HFSPLUS_SB(sb)->session;
>  		te.cdte_format = CDROM_LBA;
> -		res = ioctl_by_bdev(sb->s_bdev,
> -			CDROMREADTOCENTRY, (unsigned long)&te);
> -		if (!res && (te.cdte_ctrl & CDROM_DATA_TRACK) == 4) {
> -			*start = (sector_t)te.cdte_addr.lba << 2;
> -			return 0;
> +		if (cdrom_read_tocentry(cdi, &te) ||
> +		    (te.cdte_ctrl & CDROM_DATA_TRACK) != 4) {
> +			pr_err("invalid session number or type of track\n");
> +			return -EINVAL;
>  		}
> -		pr_err("invalid session number or type of track\n");
> -		return -EINVAL;
> +		*start = (sector_t)te.cdte_addr.lba << 2;
> +	} else if (cdi) {
> +		struct cdrom_multisession ms_info;
> +
> +		ms_info.addr_format = CDROM_LBA;
> +		if (cdrom_multisession(cdi, &ms_info) == 0 && ms_info.xa_flag)
> +			*start = (sector_t)ms_info.addr.lba << 2;
>  	}
> -	ms_info.addr_format = CDROM_LBA;
> -	res = ioctl_by_bdev(sb->s_bdev, CDROMMULTISESSION,
> -		(unsigned long)&ms_info);
> -	if (!res && ms_info.xa_flag)
> -		*start = (sector_t)ms_info.addr.lba << 2;
> +

White space change here, but I think it's ok. I do like a blank line before
return :)

>  	return 0;
>  }
>  
> 

Looks OK to me.

Reviewed-by: Damien Le Moal <damien.lemoal@....com>


-- 
Damien Le Moal
Western Digital Research