linux-kernel - Re: [PATCH 1/3] block: Return error in rescan

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: <1429671506.18798.4.camel@HansenPartnership.com>
Date:	Tue, 21 Apr 2015 19:58:26 -0700
From:	James Bottomley <James.Bottomley@...senPartnership.com>
To:	Fam Zheng <famz@...hat.com>
Cc:	linux-kernel@...r.kernel.org, Jens Axboe <axboe@...nel.dk>,
	linux-scsi@...r.kernel.org
Subject: Re: [PATCH 1/3] block: Return error in rescan_partitions if
 revalidating disk failed

On Tue, 2015-03-24 at 18:16 +0800, Fam Zheng wrote:
> If the disk can't read capacity, we should return an error.

I'm not sure I buy this: you need to justify why.

For instance removable media return an error in revalidate that causes
the medium not present flag to be set ... do we really want to propagate
the error in that case?

> Signed-off-by: Fam Zheng <famz@...hat.com>
> ---
>  block/partition-generic.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/block/partition-generic.c b/block/partition-generic.c
> index 0d9e5f9..1e60d7d 100644
> --- a/block/partition-generic.c
> +++ b/block/partition-generic.c
> @@ -427,11 +427,11 @@ rescan:
>  		return res;
>  
>  	if (disk->fops->revalidate_disk)
> -		disk->fops->revalidate_disk(disk);
> +		res = disk->fops->revalidate_disk(disk);

You're assuming here that a negative error code is being returned, but
some (cciss) seem to be returning 1 on error.

You'll need to assure us that you've checked all the consumers before
making this change.

James

>  	check_disk_size_change(disk, bdev);
>  	bdev->bd_invalidated = 0;
> -	if (!get_capacity(disk) || !(state = check_partition(disk, bdev)))
> -		return 0;
> +	if (res || !get_capacity(disk) || !(state = check_partition(disk, bdev)))
> +		return res;
>  	if (IS_ERR(state)) {
>  		/*
>  		 * I/O error reading the partition table.  If any



--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/