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 <alan@xxxxxxxxxxxxxxx> > > --- > > 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, -- To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
