Re: [PATCH] lib/string_helpers.c: fix infinite loop in string_get_size()

[James Bottomley <jbottomley@odin.com>]

[-- Attachment #1: Type: text/plain, Size: 975 bytes --]

On Thu, 2015-09-10 at 16:08 -0700, Andrew Morton wrote:
> On Fri,  4 Sep 2015 14:56:33 +0200 Vitaly Kuznetsov <vkuznets@redhat.com> wrote:
> 
> > string_get_size(1, 512, 0, ..., ...) call results in an infinite loop. The
> > problem is that if size == 0 when we start calculating sf_cap this loop
> > will never end.
> > 
> > The caller causing the issue is sd_read_capacity(), the problem was noticed
> > on Hyper-V.
> 
> When fixing bugs, please provide enough info for others to be able to
> understand which kernel version(s) need the fix.  In this case: what
> end-user action triggers this bug?  (iow, how does sdkp->capacity
> become zero?)

Any more details.  The attached programme, which is cut straight out of
the algorithm in string_helpers.c and modified for a C environment
slightly (only in do_div and the typedefs) produces this

hello
STRING IS 512 B

With your input, so I don't think the problem is where you think it is.

James


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: test.c --]
[-- Type: text/x-csrc; name="test.c", Size: 2502 bytes --]

#include <sys/types.h>
#include <string.h>
#include <stdio.h>

# define do_div(n,base) ({					\
	u32 __base = (base);				\
	u32 __rem;						\
	__rem = ((u64)(n)) % __base;			\
	(n) = ((u64)(n)) / __base;				\
	__rem;							\
 })

#define ARRAY_SIZE(arr) (sizeof(arr) / sizeof((arr)[0]))

typedef unsigned long long u64;
typedef unsigned int u32;

enum string_size_units {
  STRING_UNITS_2,
  STRING_UNITS_10,
};

/**
 * string_get_size - get the size in the specified units
 * @size:	The size to be converted in blocks
 * @blk_size:	Size of the block (use 1 for size in bytes)
 * @units:	units to use (powers of 1000 or 1024)
 * @buf:	buffer to format to
 * @len:	length of buffer
 *
 * This function returns a string formatted to 3 significant figures
 * giving the size in the required units.  @buf should have room for
 * at least 9 bytes and will always be zero terminated.
 *
 */
void string_get_size(u64 size, u64 blk_size, const enum string_size_units units,
		     char *buf, int len)
{
	static const char *const units_10[] = {
		"B", "kB", "MB", "GB", "TB", "PB", "EB", "ZB", "YB"
	};
	static const char *const units_2[] = {
		"B", "KiB", "MiB", "GiB", "TiB", "PiB", "EiB", "ZiB", "YiB"
	};
	static const char *const *const units_str[] = {
		[STRING_UNITS_10] = units_10,
		[STRING_UNITS_2] = units_2,
	};
	static const unsigned int divisor[] = {
		[STRING_UNITS_10] = 1000,
		[STRING_UNITS_2] = 1024,
	};
	int i, j;
	u32 remainder = 0, sf_cap, exp;
	char tmp[8];
	const char *unit;

	tmp[0] = '\0';
	i = 0;
	if (!size)
		goto out;

	while (blk_size >= divisor[units]) {
		remainder = do_div(blk_size, divisor[units]);
		i++;
	}

	exp = divisor[units] / (u32)blk_size;
	if (size >= exp) {
		remainder = do_div(size, divisor[units]);
		remainder *= blk_size;
		i++;
	} else {
		remainder *= size;
	}

	size *= blk_size;
	size += remainder / divisor[units];
	remainder %= divisor[units];

	while (size >= divisor[units]) {
		remainder = do_div(size, divisor[units]);
		i++;
	}

	sf_cap = size;
	for (j = 0; sf_cap*10 < 1000; j++)
		sf_cap *= 10;

	if (j) {
		remainder *= 1000;
		remainder /= divisor[units];
		snprintf(tmp, sizeof(tmp), ".%03u", remainder);
		tmp[j+1] = '\0';
	}

 out:
	if (i >= ARRAY_SIZE(units_2))
		unit = "UNK";
	else
		unit = units_str[units][i];

	snprintf(buf, len, "%u%s %s", (u32)size,
		 tmp, unit);
}

int main(char *argc[], int argv)
{
char buf[512];

printf("hello\n");
string_get_size(1, 512, STRING_UNITS_2, buf, sizeof(buf));
printf("STRING IS %s\n", buf);

}