From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1030639AbbJ3Kqe (ORCPT ); Fri, 30 Oct 2015 06:46:34 -0400 Received: from mx1.redhat.com ([209.132.183.28]:55468 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S965819AbbJ3Kqd (ORCPT ); Fri, 30 Oct 2015 06:46:33 -0400 From: Vitaly Kuznetsov To: James Bottomley Cc: "ulf.hansson\@linaro.org" , "linux\@rasmusvillemoes.dk" , "andriy.shevchenko\@linux.intel.com" , "keescook\@chromium.org" , "linux-kernel\@vger.kernel.org" , "akpm\@linux-foundation.org" Subject: Re: [PATCH v3 1/4] lib/string_helpers: change blk_size to u32 for string_get_size() interface References: <1446136250-11507-1-git-send-email-vkuznets@redhat.com> <1446136250-11507-2-git-send-email-vkuznets@redhat.com> <1446157677.25009.2.camel@Odin.com> Date: Fri, 30 Oct 2015 11:46:29 +0100 In-Reply-To: <1446157677.25009.2.camel@Odin.com> (James Bottomley's message of "Thu, 29 Oct 2015 22:27:58 +0000") Message-ID: <87d1vwskfu.fsf@vitty.brq.redhat.com> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/24.5 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org James Bottomley writes: > On Thu, 2015-10-29 at 17:30 +0100, Vitaly Kuznetsov wrote: >> string_get_size() can't really handle huge block sizes, especially >> blk_size > U32_MAX but string_get_size() interface states the opposite. >> Change blk_size from u64 to u32 to reflect the reality. > > What is the actual evidence for this? The calculation is designed to be > a symmetric 128 bit multiply. When I wrote and tested it, it worked > fine for huge block sizes. We have 'u32 remainder' and then we do: exp = divisor[units] / (u32)blk_size; ... remainder = do_div(size, divisor[units]); remainder *= blk_size; I'm pretty sure it will overflow for some inputs. > > James > >> Signed-off-by: Vitaly Kuznetsov >> --- >> include/linux/string_helpers.h | 2 +- >> lib/string_helpers.c | 4 ++-- >> 2 files changed, 3 insertions(+), 3 deletions(-) >> >> diff --git a/include/linux/string_helpers.h b/include/linux/string_helpers.h >> index dabe643..1223e80 100644 >> --- a/include/linux/string_helpers.h >> +++ b/include/linux/string_helpers.h >> @@ -10,7 +10,7 @@ enum string_size_units { >> STRING_UNITS_2, /* use binary powers of 2^10 */ >> }; >> >> -void string_get_size(u64 size, u64 blk_size, enum string_size_units units, >> +void string_get_size(u64 size, u32 blk_size, enum string_size_units units, >> char *buf, int len); >> >> #define UNESCAPE_SPACE 0x01 >> diff --git a/lib/string_helpers.c b/lib/string_helpers.c >> index 5939f63..f6c27dc 100644 >> --- a/lib/string_helpers.c >> +++ b/lib/string_helpers.c >> @@ -26,7 +26,7 @@ >> * 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, >> +void string_get_size(u64 size, u32 blk_size, const enum string_size_units units, >> char *buf, int len) >> { >> static const char *const units_10[] = { >> @@ -58,7 +58,7 @@ void string_get_size(u64 size, u64 blk_size, const enum string_size_units units, >> i++; >> } >> >> - exp = divisor[units] / (u32)blk_size; >> + exp = divisor[units] / blk_size; >> /* >> * size must be strictly greater than exp here to ensure that remainder >> * is greater than divisor[units] coming out of the if below. -- Vitaly