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 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
