On Fri, 21 Oct 2005, James Bottomley wrote:
> On Thu, 2005-10-20 at 20:55 -0700, Randy.Dunlap wrote:
> > With CONFIG_LBD=n, 'sz' (32 bits) can overflow when capacity is
> > multiplied (even * 2), as seen in a report from Dale Blount
> > on lkml. Also make sure that 'mb' will not overflow.
> >
> > Signed-off-by: Randy Dunlap <rdunlap@xxxxxxxxxxxx>
>
> Mathematically, it doesn't look like we need to use 64 bits at all;
> what's wrong with the attached? It looks like the code once allowed 256
> byte sectors, but the if statement above now forbids them.
What if statement forbids 256-byte sectors? I don't see it.
if (sector_size != 512 &&
sector_size != 1024 &&
sector_size != 2048 &&
sector_size != 4096 &&
sector_size != 256) {
printk(KERN_NOTICE "%s : unsupported sector size "
"%d.\n", diskname, sector_size);
allows sector_size of 256.
> I'm really not terribly sympathetic to anyone trying Terrabyte discs on
> a non-LBD 32 bit kernel. The potential for overflow (and consequent
> data loss) is pretty huge.
Yes, I agree with that.
> James
>
> diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
> --- a/drivers/scsi/sd.c
> +++ b/drivers/scsi/sd.c
> @@ -1253,14 +1253,13 @@ got_data:
> * Jacques Gelinas (Jacques@xxxxxxxxxxxxxx)
> */
> int hard_sector = sector_size;
> - sector_t sz = sdkp->capacity * (hard_sector/256);
> + sector_t sz = sdkp->capacity * (hard_sector/512);
> request_queue_t *queue = sdp->request_queue;
> - sector_t mb;
> + sector_t mb = sz;
>
> blk_queue_hardsect_size(queue, hard_sector);
> /* avoid 64-bit division on 32-bit platforms */
> - mb = sz >> 1;
> - sector_div(sz, 1250);
> + sector_div(sz, 625);
> mb -= sz - 974;
> sector_div(mb, 1950);
>
>
>
>
--
~Randy
-
: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
