| 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
| ||
|
Message-ID: <gpuskwq7kwwf52abwrh6my462cnlhorpohpk6a5wzya4qdyvdb@gqibcr57zkv3>
Date: Thu, 29 Feb 2024 16:45:42 -0500
From: Kent Overstreet <kent.overstreet@...ux.dev>
To: Andy Shevchenko <andriy.shevchenko@...ux.intel.com>
Cc: Kees Cook <keescook@...omium.org>, linux-hardening@...r.kernel.org,
linux-kernel@...r.kernel.org, Andy Shevchenko <andy@...nel.org>,
Andrew Morton <akpm@...ux-foundation.org>, Suren Baghdasaryan <surenb@...gle.com>
Subject: Re: [PATCH v1 1/1] lib/string_helpers: Add flags param to
string_get_size()
On Thu, Feb 29, 2024 at 10:52:30PM +0200, Andy Shevchenko wrote:
> From: Kent Overstreet <kent.overstreet@...ux.dev>
>
> The new flags parameter allows controlling
> - Whether or not the units suffix is separated by a space, for
> compatibility with sort -h
> - Whether or not to append a B suffix - we're not always printing
> bytes.
>
> Signed-off-by: Kent Overstreet <kent.overstreet@...ux.dev>
> Signed-off-by: Suren Baghdasaryan <surenb@...gle.com>
> Reviewed-by: Kees Cook <keescook@...omium.org>
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@...ux.intel.com>
You shouldn't have included my SOB after making your own changes - this
all looks fine, though.
Reviewed-by: Kent Overstreet <kent.overstreet@...ux.dev>
Also, does this mean you're picking this patch up? We're actually
dropping it from our patchset, but I'd still like to get it in, it's
just a patch routing question now. I'd use it for other stuff, so I can
add it to another pull request if you prefer.
> ---
>
> My vesrion of https://lore.kernel.org/r/20240212213922.783301-2-surenb@google.com
> Enjoy!
>
> include/linux/string_helpers.h | 10 ++++--
> lib/string_helpers.c | 29 ++++++++-------
> lib/test-string_helpers.c | 65 ++++++++++++++++++++++++++++------
> 3 files changed, 78 insertions(+), 26 deletions(-)
>
> diff --git a/include/linux/string_helpers.h b/include/linux/string_helpers.h
> index 58fb1f90eda5..e93fbb5b0c01 100644
> --- a/include/linux/string_helpers.h
> +++ b/include/linux/string_helpers.h
> @@ -17,14 +17,18 @@ static inline bool string_is_terminated(const char *s, int len)
> return memchr(s, '\0', len) ? true : false;
> }
>
> -/* Descriptions of the types of units to
> - * print in */
> +/* Descriptions of the types of units to print in */
> enum string_size_units {
> STRING_UNITS_10, /* use powers of 10^3 (standard SI) */
> STRING_UNITS_2, /* use binary powers of 2^10 */
> + STRING_UNITS_MASK = BIT(0),
> +
> + /* Modifiers */
> + STRING_UNITS_NO_SPACE = BIT(30),
> + STRING_UNITS_NO_BYTES = BIT(31),
> };
>
> -int string_get_size(u64 size, u64 blk_size, enum string_size_units units,
> +int string_get_size(u64 size, u64 blk_size, const enum string_size_units units,
> char *buf, int len);
>
> int parse_int_array_user(const char __user *from, size_t count, int **array);
> diff --git a/lib/string_helpers.c b/lib/string_helpers.c
> index 6bbafd6a10d9..69ba49b853c7 100644
> --- a/lib/string_helpers.c
> +++ b/lib/string_helpers.c
> @@ -25,7 +25,7 @@
> * 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)
> + * @units: Units to use (powers of 1000 or 1024), whether to include space separator
> * @buf: buffer to format to
> * @len: length of buffer
> *
> @@ -39,11 +39,12 @@
> int string_get_size(u64 size, u64 blk_size, const enum string_size_units units,
> char *buf, int len)
> {
> + enum string_size_units units_base = units & STRING_UNITS_MASK;
> static const char *const units_10[] = {
> - "B", "kB", "MB", "GB", "TB", "PB", "EB", "ZB", "YB"
> + "", "k", "M", "G", "T", "P", "E", "Z", "Y",
> };
> static const char *const units_2[] = {
> - "B", "KiB", "MiB", "GiB", "TiB", "PiB", "EiB", "ZiB", "YiB"
> + "", "Ki", "Mi", "Gi", "Ti", "Pi", "Ei", "Zi", "Yi",
> };
> static const char *const *const units_str[] = {
> [STRING_UNITS_10] = units_10,
> @@ -68,7 +69,7 @@ int string_get_size(u64 size, u64 blk_size, const enum string_size_units units,
>
> /* This is Napier's algorithm. Reduce the original block size to
> *
> - * coefficient * divisor[units]^i
> + * coefficient * divisor[units_base]^i
> *
> * we do the reduction so both coefficients are just under 32 bits so
> * that multiplying them together won't overflow 64 bits and we keep
> @@ -78,12 +79,12 @@ int string_get_size(u64 size, u64 blk_size, const enum string_size_units units,
> * precision is in the coefficients.
> */
> while (blk_size >> 32) {
> - do_div(blk_size, divisor[units]);
> + do_div(blk_size, divisor[units_base]);
> i++;
> }
>
> while (size >> 32) {
> - do_div(size, divisor[units]);
> + do_div(size, divisor[units_base]);
> i++;
> }
>
> @@ -92,8 +93,8 @@ int string_get_size(u64 size, u64 blk_size, const enum string_size_units units,
> size *= blk_size;
>
> /* and logarithmically reduce it until it's just under the divisor */
> - while (size >= divisor[units]) {
> - remainder = do_div(size, divisor[units]);
> + while (size >= divisor[units_base]) {
> + remainder = do_div(size, divisor[units_base]);
> i++;
> }
>
> @@ -103,10 +104,10 @@ int string_get_size(u64 size, u64 blk_size, const enum string_size_units units,
> for (j = 0; sf_cap*10 < 1000; j++)
> sf_cap *= 10;
>
> - if (units == STRING_UNITS_2) {
> + if (units_base == STRING_UNITS_2) {
> /* express the remainder as a decimal. It's currently the
> * numerator of a fraction whose denominator is
> - * divisor[units], which is 1 << 10 for STRING_UNITS_2 */
> + * divisor[units_base], which is 1 << 10 for STRING_UNITS_2 */
> remainder *= 1000;
> remainder >>= 10;
> }
> @@ -128,10 +129,12 @@ int string_get_size(u64 size, u64 blk_size, const enum string_size_units units,
> if (i >= ARRAY_SIZE(units_2))
> unit = "UNK";
> else
> - unit = units_str[units][i];
> + unit = units_str[units_base][i];
>
> - return snprintf(buf, len, "%u%s %s", (u32)size,
> - tmp, unit);
> + return snprintf(buf, len, "%u%s%s%s%s", (u32)size, tmp,
> + (units & STRING_UNITS_NO_SPACE) ? "" : " ",
> + unit,
> + (units & STRING_UNITS_NO_BYTES) ? "" : "B");
> }
> EXPORT_SYMBOL(string_get_size);
>
> diff --git a/lib/test-string_helpers.c b/lib/test-string_helpers.c
> index 9a68849a5d55..dce67698297b 100644
> --- a/lib/test-string_helpers.c
> +++ b/lib/test-string_helpers.c
> @@ -3,6 +3,7 @@
> */
> #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
>
> +#include <linux/array_size.h>
> #include <linux/init.h>
> #include <linux/kernel.h>
> #include <linux/slab.h>
> @@ -500,21 +501,65 @@ static __init void test_string_get_size_check(const char *units,
> pr_warn("expected: '%s', got '%s'\n", exp, res);
> }
>
> +static __init void __strchrcut(char *dst, const char *src, const char *cut)
> +{
> + const char *from = src;
> + size_t len;
> +
> + do {
> + len = strcspn(from, cut);
> + memcpy(dst, from, len);
> + dst += len;
> + from += len;
> + } while (*from++);
> + *dst = '\0';
> +}
> +
> +static __init void __test_string_get_size_one(const u64 size, const u64 blk_size,
> + const char *exp_result10,
> + const char *exp_result2,
> + enum string_size_units units,
> + const char *cut)
> +{
> + char buf10[string_get_size_maxbuf];
> + char buf2[string_get_size_maxbuf];
> + char exp10[string_get_size_maxbuf];
> + char exp2[string_get_size_maxbuf];
> + char prefix10[64];
> + char prefix2[64];
> +
> + sprintf(prefix10, "STRING_UNITS_10 [%s]", cut);
> + sprintf(prefix2, "STRING_UNITS_2 [%s]", cut);
> +
> + __strchrcut(exp10, exp_result10, cut);
> + __strchrcut(exp2, exp_result2, cut);
> +
> + string_get_size(size, blk_size, STRING_UNITS_10 | units, buf10, sizeof(buf10));
> + string_get_size(size, blk_size, STRING_UNITS_2 | units, buf2, sizeof(buf2));
> +
> + test_string_get_size_check(prefix10, exp10, buf10, size, blk_size);
> + test_string_get_size_check(prefix2, exp2, buf2, size, blk_size);
> +}
> +
> static __init void __test_string_get_size(const u64 size, const u64 blk_size,
> const char *exp_result10,
> const char *exp_result2)
> {
> - char buf10[string_get_size_maxbuf];
> - char buf2[string_get_size_maxbuf];
> + struct {
> + enum string_size_units units;
> + const char *cut;
> + } get_size_test_cases[] = {
> + { 0, "" },
> + { STRING_UNITS_NO_SPACE, " " },
> + { STRING_UNITS_NO_SPACE | STRING_UNITS_NO_BYTES, " B" },
> + { STRING_UNITS_NO_BYTES, "B" },
> + };
> + int i;
>
> - string_get_size(size, blk_size, STRING_UNITS_10, buf10, sizeof(buf10));
> - string_get_size(size, blk_size, STRING_UNITS_2, buf2, sizeof(buf2));
> -
> - test_string_get_size_check("STRING_UNITS_10", exp_result10, buf10,
> - size, blk_size);
> -
> - test_string_get_size_check("STRING_UNITS_2", exp_result2, buf2,
> - size, blk_size);
> + for (i = 0; i < ARRAY_SIZE(get_size_test_cases); i++)
> + __test_string_get_size_one(size, blk_size, exp_result10, exp_result2,
> + get_size_test_cases[i].units,
> + get_size_test_cases[i].cut);
> }
>
> static __init void test_string_get_size(void)
> --
> 2.43.0.rc1.1.gbec44491f096
>
Powered by blists - more mailing lists