linux-kernel - Re: [PATCH] sd: close hole in > 2T device rejection when !CONFIG

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:   Mon, 27 Feb 2017 16:13:43 +0000
From:   Bart Van Assche <Bart.VanAssche@...disk.com>
To:     "jejb@...ux.vnet.ibm.com" <jejb@...ux.vnet.ibm.com>,
        "steve.magnani@...idescorp.com" <steve.magnani@...idescorp.com>,
        "martin.petersen@...cle.com" <martin.petersen@...cle.com>
CC:     "linux-scsi@...r.kernel.org" <linux-scsi@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "steve@...idescorp.com" <steve@...idescorp.com>
Subject: Re: [PATCH] sd: close hole in > 2T device rejection when
 !CONFIG_LBDAF

On Mon, 2017-02-27 at 09:22 -0600, Steven J. Magnani wrote:
> @@ -2122,7 +2122,10 @@ static int read_capacity_16(struct scsi_
>  		return -ENODEV;
>  	}
>  
> -	if ((sizeof(sdkp->capacity) == 4) && (lba >= 0xffffffffULL)) {
> +	/* Make sure logical_to_sectors() won't overflow */
> +	lba_in_sectors = lba << (ilog2(sector_size) - 9);
> +	if ((sizeof(sdkp->capacity) == 4) &&
> +	    ((lba >= 0xffffffffULL) || (lba_in_sectors >= 0xffffffffULL))) {
>  		sd_printk(KERN_ERR, sdkp, "Too big for this kernel. Use a "
>  			"kernel compiled with support for large block "
>  			"devices.\n");
> @@ -2162,6 +2165,7 @@ static int read_capacity_10(struct scsi_
>  	int the_result;
>  	int retries = 3, reset_retries = READ_CAPACITY_RETRIES_ON_RESET;
>  	sector_t lba;
> +	unsigned long long lba_in_sectors;
>  	unsigned sector_size;
>  
>  	do {
> @@ -2208,7 +2212,10 @@ static int read_capacity_10(struct scsi_
>  		return sector_size;
>  	}
>  
> -	if ((sizeof(sdkp->capacity) == 4) && (lba == 0xffffffff)) {
> +	/* Make sure logical_to_sectors() won't overflow */
> +	lba_in_sectors = ((unsigned long long) lba) << (ilog2(sector_size) - 9);
> +	if ((sizeof(sdkp->capacity) == 4) &&
> +	    (lba_in_sectors >= 0xffffffffULL)) {
>  		sd_printk(KERN_ERR, sdkp, "Too big for this kernel. Use a "
>  			"kernel compiled with support for large block "
>  			"devices.\n");

Why are the two checks slightly different? Could the same code be used for
both checks? BTW, using the macro below would make the above checks less
verbose and easier to read:

/*
 * Test whether the result of a shift-left operation would be larger than
 * what fits in a variable with the type of @a.
 */
#define shift_left_overflows(a, b)					\
	({								\
		typeof(a) _minus_one = -1LL;				\
		typeof(a) _plus_one = 1;				\
		bool _a_is_signed = _minus_one < 0;			\
		int _shift = sizeof(a) * 8 - ((b) + _a_is_signed);	\
		_shift < 0 || ((a) & ~((_plus_one << _shift) - 1)) != 0;\
	})

Bart.