Re: [PATCH] sd: close hole in > 2T device rejection when !CONFIG_LBDAF

[Bart Van Assche <Bart.VanAssche@sandisk.com>]

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.