linux-kernel - Re: [RFC] block: check partition alignment

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, 15 Dec 2016 10:33:47 +0900
From:   Damien Le Moal <damien.lemoal@....com>
To:     Christoph Hellwig <hch@...radead.org>,
        Stefan Haberland <sth@...ux.vnet.ibm.com>
Cc:     axboe@...nel.dk, linux-block@...r.kernel.org,
        linux-kernel@...r.kernel.org, hoeppner@...ux.vnet.ibm.com,
        sebott@...ux.vnet.ibm.com
Subject: Re: [RFC] block: check partition alignment

Christoph,

On 12/15/16 02:07, Christoph Hellwig wrote:
>> To prevent partitions that are not aligned to the physical blocksize
>> of a device check for the alignment in the blkpg_ioctl.
> 
> We'd also need to reject this when reading partitions from disk, right?

Only for DASD devices, no ?

Logical block size aligned partitions are fine for regular block
devices. Not aligning on the physical block size is indeed very stupid,
but will not generate errors and an application can see that through
bdev_alignment_offset() and the sysfs alignment_offset file of the
partition.

> 
>> +			/* check if partition is aligned to blocksize */
>> +			if (p.start % bdev_physical_block_size(bdev) != 0)
> 
> And this should be bdev_logical_block_size, as the logical block size
> is the only thing that matters for the OS - exposing the physical block
> size is just an optional hint to prevent users from doing stupid
> things (like creating unaligned partitions :))

For a regular block device, I agree. But in Stephan case, I think that
the check really needs to be against the physical block size, with the
added condition that the bdev is a DASD device (similarly to the zone
alignment check for zoned block devices).

So this should become something like:

if (p.start & (bdev_logical_block_size(bdev) - 1))
	return -EINVAL;
if (bdev_is_dasd(bdev) &&
    p.start & (bdev_physical_block_size(bdev) - 1))
	return -EINVAL;

I am not sure however how bdev_is_dasd can be implemented though.

Best regards.

-- 
Damien Le Moal, Ph.D.
Sr. Manager, System Software Research Group,
Western Digital Corporation
Damien.LeMoal@....com
(+81) 0466-98-3593 (ext. 513593)
1 kirihara-cho, Fujisawa,
Kanagawa, 252-0888 Japan
www.wdc.com, www.hgst.com