linux-kernel - Re: [PATCH v3 2/3] block: add a s390-only biodasdinfo method

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]

Message-ID: <20200508155306.GB4200@lst.de>
Date:   Fri, 8 May 2020 17:53:06 +0200
From:   Christoph Hellwig <hch@....de>
To:     Stefan Haberland <sth@...ux.ibm.com>
Cc:     hch@....de, axboe@...nel.dk, hoeppner@...ux.ibm.com,
        linux-s390@...r.kernel.org, heiko.carstens@...ibm.com,
        gor@...ux.ibm.com, linux-kernel@...r.kernel.org,
        borntraeger@...ibm.com
Subject: Re: [PATCH v3 2/3] block: add a s390-only biodasdinfo method

On Fri, May 08, 2020 at 03:14:54PM +0200, Stefan Haberland wrote:
> From: Christoph Hellwig <hch@....de>
> 
> The IBM partition parser needs to query the DASD driver for details that
> are very s390 specific.  Instead of using ioctl_by_bdev with a fake user
> space pointer just add a s390-specific method to get the information
> directly.
> 
> Signed-off-by: Christoph Hellwig <hch@....de>
> [sth@...ux.ibm.com: remove fop, add gendisk check, export funcion]
> Signed-off-by: Stefan Haberland <sth@...ux.ibm.com>

The subject and changelog need updates for your changes.  I think you
should also claim authorship, even if a few bits are originally from me.
Probaby it would make sense to even just merge this into the next patch.

> index fa552f9f1666..6eac7b11c75b 100644
> --- a/drivers/s390/block/dasd_int.h
> +++ b/drivers/s390/block/dasd_int.h
> @@ -845,6 +845,7 @@ void dasd_destroy_partitions(struct dasd_block *);
>  
>  /* externals in dasd_ioctl.c */
>  int  dasd_ioctl(struct block_device *, fmode_t, unsigned int, unsigned long);
> +int dasd_biodasdinfo(struct gendisk *disk, struct dasd_information2_t *info);

I think this needs to go to a public include/linux/ header for the
partitioning code to share the prototype.

> +int dasd_biodasdinfo(struct gendisk *disk, struct dasd_information2_t *info)
> +{
> +	struct dasd_device *base;
> +	int error;
> +
> +	/*
> +	 * we might get called externaly, so check if the gendisk belongs
> +	 * to a DASD by checking the fops pointer
> +	 */
> +	if (disk->fops != &dasd_device_operations)
> +		return -EINVAL;

I think a function comment (e.g. kernel doc) explaining the use case and
this detail might be useful.