From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932247AbbJaAHZ (ORCPT ); Fri, 30 Oct 2015 20:07:25 -0400 Received: from mx2.parallels.com ([199.115.105.18]:55503 "EHLO mx2.parallels.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752455AbbJaAHY (ORCPT ); Fri, 30 Oct 2015 20:07:24 -0400 From: James Bottomley To: "vkuznets@redhat.com" CC: "andy.shevchenko@gmail.com" , "ulf.hansson@linaro.org" , "linux@rasmusvillemoes.dk" , "andriy.shevchenko@linux.intel.com" , "linux-kernel@vger.kernel.org" , "akpm@linux-foundation.org" , "keescook@chromium.org" Subject: Re: [PATCH v3 2/4] lib/string_helpers.c: protect string_get_size() against blk_size=0 Thread-Topic: [PATCH v3 2/4] lib/string_helpers.c: protect string_get_size() against blk_size=0 Thread-Index: AQHREv+Duq98ZTg6HkG6bAUDyFTYep6FL4gA Date: Sat, 31 Oct 2015 00:07:13 +0000 Message-ID: <1446250032.25009.44.camel@Odin.com> References: <1446136250-11507-1-git-send-email-vkuznets@redhat.com> <1446136250-11507-3-git-send-email-vkuznets@redhat.com> <1446159638.25009.5.camel@Odin.com> <1446176044.25009.17.camel@Odin.com> <87h9l8skoe.fsf@vitty.brq.redhat.com> In-Reply-To: <87h9l8skoe.fsf@vitty.brq.redhat.com> Accept-Language: en-GB, en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-mailer: Evolution 3.12.11 x-ms-exchange-transport-fromentityheader: Hosted x-originating-ip: [61.115.201.89] Content-Type: text/plain; charset="utf-8" Content-ID: <22BB69C0FDE4194195840AE98D31D0DA@sw.swsoft.com> MIME-Version: 1.0 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Transfer-Encoding: 8bit X-MIME-Autoconverted: from base64 to 8bit by mail.home.local id t9V07Tju023797 On Fri, 2015-10-30 at 11:41 +0100, Vitaly Kuznetsov wrote: > James Bottomley writes: > > > On Fri, 2015-10-30 at 01:32 +0200, Andy Shevchenko wrote: > >> On Fri, Oct 30, 2015 at 1:00 AM, James Bottomley wrote: > >> > On Thu, 2015-10-29 at 17:30 +0100, Vitaly Kuznetsov wrote: > >> >> Division by zero happens if blk_size=0 is supplied to string_get_size(). > >> >> Add WARN_ON() and set size to 0 to report '0 B'. > >> >> > >> >> Signed-off-by: Vitaly Kuznetsov > >> >> --- > >> >> lib/string_helpers.c | 5 +++++ > >> >> 1 file changed, 5 insertions(+) > >> >> > >> >> diff --git a/lib/string_helpers.c b/lib/string_helpers.c > >> >> index f6c27dc..ff3575b 100644 > >> >> --- a/lib/string_helpers.c > >> >> +++ b/lib/string_helpers.c > >> >> @@ -50,6 +50,11 @@ void string_get_size(u64 size, u32 blk_size, const enum string_size_units units, > >> >> > >> >> tmp[0] = '\0'; > >> >> i = 0; > >> >> + > >> >> + /* Calling string_get_size() with blk_size=0 is wrong! */ > >> >> + if (WARN_ON(!blk_size)) > >> > > >> > Get rid of the WARN_ON; it's the standard thing to do for a partially > >> > connected device. Seeing zero is standard in a whole variety of > >> > situations. SCSI shims the zero but most other drivers don't. > >> > >> For *block* size? It will crash the kernel. I've checked, it wasn't > >> changed from the beginning (b9f28d863594). > > > > The standard signal for a drive error in capacity is zero size and zero > > block size. We have to take that case as standard without emitting > > scary warnings. > > Ok, but what if size != 0? Is WARN_ON() justified in this case? It's an arithmentic routine whose job is to multiply two numbers, not second guess the subsystem that gave it the numbers. Just on general architectural principles the only time it's allowed to dump a stack trace without confusing people is when the arithmetic operation it has been asked to do would produce an illegal result (which for two sixty four bit numbers multiplying to a 128 bit one is never). James {.n++%ݶw{.n+{G{ayʇڙ,jfhz_(階ݢj"mG?&~iOzv^m ?I