linux-kernel - Re: [PATCH 4.19 13/35] null_blk: Fix zone size initialization

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] [day] [month] [year] [list]

Date:   Mon, 11 Jan 2021 02:46:57 +0000
From:   Damien Le Moal <Damien.LeMoal@....com>
To:     Pavel Machek <pavel@....cz>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>
CC:     "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "stable@...r.kernel.org" <stable@...r.kernel.org>,
        Naohiro Aota <Naohiro.Aota@....com>,
        Christoph Hellwig <hch@....de>,
        Johannes Thumshirn <Johannes.Thumshirn@....com>,
        Jens Axboe <axboe@...nel.dk>
Subject: Re: [PATCH 4.19 13/35] null_blk: Fix zone size initialization

On 2021/01/06 21:55, Pavel Machek wrote:
> Hi!
> 
>> commit 0ebcdd702f49aeb0ad2e2d894f8c124a0acc6e23 upstream.
>>
>> For a null_blk device with zoned mode enabled is currently initialized
>> with a number of zones equal to the device capacity divided by the zone
>> size, without considering if the device capacity is a multiple of the
>> zone size. If the zone size is not a divisor of the capacity, the zones
>> end up not covering the entire capacity, potentially resulting is out
>> of bounds accesses to the zone array.
>>
>> Fix this by adding one last smaller zone with a size equal to the
>> remainder of the disk capacity divided by the zone size if the capacity
>> is not a multiple of the zone size. For such smaller last zone, the zone
>> capacity is also checked so that it does not exceed the smaller zone
>> size.
> 
>> --- a/drivers/block/null_blk_zoned.c
>> +++ b/drivers/block/null_blk_zoned.c
>> @@ -1,9 +1,9 @@
>>  // SPDX-License-Identifier: GPL-2.0
>>  #include <linux/vmalloc.h>
>> +#include <linux/sizes.h>
>>  #include "null_blk.h"
>>  
>> -/* zone_size in MBs to sectors. */
>> -#define ZONE_SIZE_SHIFT		11
>> +#define MB_TO_SECTS(mb) (((sector_t)mb * SZ_1M) >> SECTOR_SHIFT)
> 
> This macro is quite dangerous. (mb) would help, but inline function
> would be better.

Indeed.

> 
> 
>> +	dev->nr_zones = dev_capacity_sects >> ilog2(dev->zone_size_sects);
>> +	if (dev_capacity_sects & (dev->zone_size_sects - 1))
>> +		dev->nr_zones++;
> 
> Is this same as nr_zones = DIV_ROUND_UP(dev_capacity_sects,
> dev->zone_size_sects)? Would that be faster, more readable and robust
> against weird dev->zone_size_sects sizes?

Yes, we can change to this to be more readable.
Will send a cleanup patch. Thanks !

> 
> Best regards,
> 								Pavel
> 


-- 
Damien Le Moal
Western Digital Research