From mboxrd@z Thu Jan 1 00:00:00 1970 From: Bartlomiej Zolnierkiewicz Subject: Re: [PATCH] libata: Fix division by zero Date: Wed, 08 Apr 2015 16:03:23 +0200 Message-ID: <2788063.ZcUqKDEcli@amdc1032> References: <20150408125114.29925.841.stgit@localhost.localdomain> <5105249.IaGgeTGhXs@amdc1032> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7Bit Return-path: Received: from mailout3.samsung.com ([203.254.224.33]:8469 "EHLO mailout3.samsung.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753438AbbDHODk (ORCPT ); Wed, 8 Apr 2015 10:03:40 -0400 Received: from epcpsbgm1.samsung.com (epcpsbgm1 [203.254.230.26]) by mailout3.samsung.com (Oracle Communications Messaging Server 7.0.5.31.0 64bit (built May 5 2014)) with ESMTP id <0NMH00H4GR225QA0@mailout3.samsung.com> for linux-ide@vger.kernel.org; Wed, 08 Apr 2015 23:03:38 +0900 (KST) In-reply-to: <5105249.IaGgeTGhXs@amdc1032> Sender: linux-ide-owner@vger.kernel.org List-Id: linux-ide@vger.kernel.org To: Alan Cc: tj@kernel.org, linux-ide@vger.kernel.org On Wednesday, April 08, 2015 03:52:58 PM Bartlomiej Zolnierkiewicz wrote: > > Hi, > > On Wednesday, April 08, 2015 01:51:36 PM Alan wrote: > > (Coverity 1192289, 1192292, 1192294) > > > > We have several controllers that in some cases use ata_timing_compute but do > > not do UDMA. They pass 0 for UT, which ends up with us doing a division by > > zero. We could pass some other bogus value in or we could make the libata > > code do the sensible thing and treat a UT of 0 as meaning "I'm not asking > > about UDMA". > > > > This patches does the latter which is IMHO the more robust option. > > Please note that we should not end up with the division by zero with > existing host drivers. EZ(v, unit) checks for v being zero and t->udma > is non-zero only for XFER_UDMA_* xfer modes. Currently none of host A bit more detailed explanation for xfer_modes < XFER_UDMA_0: #define ENOUGH(v, unit) (((v)-1)/(unit)+1) #define EZ(v, unit) ((v)?ENOUGH(v, unit):0) q->udma = EZ(t->udma * 1000, UT); for xfer modes < XFER_UDMA_0 t->udma is always zero so EZ() will return zero without any division. Best regards, -- Bartlomiej Zolnierkiewicz Samsung R&D Institute Poland Samsung Electronics > drivers calls ata_timing_compute() on xfer mode >= XFER_UDMA_0 with > the zero UT argument. Such usage will be an error and while the old > code explodes immediately, the new one will silently accept it (which > could theoretically result in the wrong timing being set).. > > Could you please enhance your patch with a validity code requiring > non-zero UT when speed >= XFER_UDMA_0 is used in ata_timing_compute()? > > Best regards, > -- > Bartlomiej Zolnierkiewicz > Samsung R&D Institute Poland > Samsung Electronics > > > Signed-off-by: Alan Cox > > --- > > 0 files changed > > > > diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c > > index f6cb1f1..6b1bd36 100644 > > --- a/drivers/ata/libata-core.c > > +++ b/drivers/ata/libata-core.c > > @@ -2966,7 +2966,8 @@ static void ata_timing_quantize(const struct ata_timing *t, struct ata_timing *q > > q->recover = EZ(t->recover * 1000, T); > > q->dmack_hold = EZ(t->dmack_hold * 1000, T); > > q->cycle = EZ(t->cycle * 1000, T); > > - q->udma = EZ(t->udma * 1000, UT); > > + if (UT) > > + q->udma = EZ(t->udma * 1000, UT); > > } > > > > void ata_timing_merge(const struct ata_timing *a, const struct ata_timing *b,