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

James Bottomley
2015-09-11T18:31:15Z

[-- 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);

}