linux-kernel - Re: [PATCH 1/2] dm: Set safe default max_sectors for targets with no underlying device

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 for Android: free password hash cracker in your pocket

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

Message-ID: <20090918155217.GA28926@redhat.com>
Date:	Fri, 18 Sep 2009 11:52:17 -0400
From:	Mike Snitzer <snitzer@...hat.com>
To:	"Jun'ichi Nomura" <j-nomura@...jp.nec.com>
Cc:	Jens Axboe <jens.axboe@...cle.com>,
	Alasdair G Kergon <agk@...hat.com>,
	"Martin K. Petersen" <martin.petersen@...cle.com>,
	David Strand <dpstrand@...il.com>,
	device-mapper development <dm-devel@...hat.com>,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH 1/2] dm: Set safe default max_sectors for targets with
	no underlying device

On Fri, Sep 18 2009 at 11:18am -0400,
Jun'ichi Nomura <j-nomura@...jp.nec.com> wrote:

> This is a preparation for the next patch, which changes
> blk_set_default_limits() to set 0 for max_sectors/max_hw_sectors.
> With the next patch, for dm targets like 'zero', where there's no
> underlying device, those values remain 0 and the dm device becomes
> unusable.
> 
> So check the max_sectors and set to SAFE_MAX_SECTORS if 0.
> 
> Check this thread for further background:
> https://www.redhat.com/archives/dm-devel/2009-September/msg00176.html
> 
> 
> Signed-off-by: Kiyoshi Ueda <k-ueda@...jp.nec.com>
> Signed-off-by: Jun'ichi Nomura <j-nomura@...jp.nec.com>
> Cc: David Strand <dpstrand@...il.com>
> Cc: Mike Snitzer <snitzer@...hat.com>
> Cc: Alasdair G Kergon <agk@...hat.com>
> Cc: Martin K. Petersen <martin.petersen@...cle.com>
> Cc: Jens Axboe <jens.axboe@...cle.com>
> ---
>  drivers/md/dm-table.c |   11 +++++++++++
>  1 file changed, 11 insertions(+)
> 
> Index: linux-2.6.31.work/drivers/md/dm-table.c
> ===================================================================
> --- linux-2.6.31.work.orig/drivers/md/dm-table.c
> +++ linux-2.6.31.work/drivers/md/dm-table.c
> @@ -707,6 +707,17 @@ static int validate_hardware_logical_blo
>  		    device_logical_block_size_sects - next_target_start : 0;
>  	}
> 
> +	/*
> +	 * blk_set_default_limits() sets max_sectors/max_hw_sectors to 0.
> +	 * When all targets have no underlying device, they are
> +	 * left unchanged from the default values and cause problems.
> +	 * Use SAFE_MAX_SECTORS for such cases.
> +	 */
> +	if (limits->max_hw_sectors == 0)
> +		limits->max_hw_sectors = SAFE_MAX_SECTORS;
> +	if (limits->max_sectors == 0)
> +		limits->max_sectors = SAFE_MAX_SECTORS;
> +
>  	if (remaining) {
>  		DMWARN("%s: table line %u (start sect %llu len %llu) "
>  		       "not aligned to h/w logical block size %u",

The blk_set_default_limits() that matters, as referenced in your comment
block, is actually in dm_calculate_queue_limits().

It would be better to put these checks in dm_calculate_queue_limits()
just before the call to validate_hardware_logical_block_alignment().

Either way works because the checks are effectively at the end of
dm_calculate_queue_limits() -- which is where they need to be.

But I think you're polluting validate_hardware_logical_block_alignment()
with unrelated checks.

Thanks,
Mike
--
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/