Re: PR/47879 CVS commit: src/sys/dev

[Takahiro HAYASHI <t-hash%abox3.so-net.ne.jp@localhost>]

On Mon,  3 Jun 2013 16:45:01 +0000 (UTC)
"Christos Zoulas" <christos%netbsd.org@localhost> wrote:

> The following reply was made to PR kern/47879; it has been noted by GNATS.
> 
> From: "Christos Zoulas" <christos%netbsd.org@localhost>
> To: gnats-bugs%gnats.NetBSD.org@localhost
> Cc: 
> Subject: PR/47879 CVS commit: src/sys/dev
> Date: Mon, 3 Jun 2013 12:42:32 -0400
> 
>  Module Name: src
>  Committed By:        christos
>  Date:                Mon Jun  3 16:42:32 UTC 2013
>  
>  Modified Files:
>       src/sys/dev: vnd.c vndvar.h
>  
>  Log Message:
>  PR/47879: Takahiro HAYASHI: vnd cannot handle disk image larger than 2TiB
>  change size_t to uint64_t where needed.

Thanks for commit but the fix is insufficient.

In function vnd_set_geometry() of src/sys/dev/vnd.c, sectors per unit
is the product of num of sectors, tracks and cylinders.
This may cause interger overflow.

        dg->dg_secperunit = vnd->sc_geom.vng_nsectors *
            vnd->sc_geom.vng_ntracks * vnd->sc_geom.vng_ncylinders;

The typeof right side variables are all uint32_t, so the result is
calculated in interger.

One of solutions to avoid overflow is to start right side
with "1LL *".

        dg->dg_secperunit = 1LL * vnd->sc_geom.vng_nsectors *
            vnd->sc_geom.vng_ntracks * vnd->sc_geom.vng_ncylinders;

Or cast first variable to int64_t.

        dg->dg_secperunit = (int64_t)vnd->sc_geom.vng_nsectors *
            vnd->sc_geom.vng_ntracks * vnd->sc_geom.vng_ncylinders;

--
t-hash