lkml.org 
[lkml]   [2006]   [May]   [18]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH] sector_t overflow in block layer
    On Thu, 18 May 2006, Linus Torvalds wrote:
    > On Thu, 18 May 2006, Andreas Dilger wrote:
    > > struct bio *bio;
    > > + unsigned long long sector;
    > > int ret = 0;
    > >
    > > BUG_ON(!buffer_locked(bh));
    > > BUG_ON(!buffer_mapped(bh));
    > > BUG_ON(!bh->b_end_io);
    > >
    > > + /* Check if we overflow sector_t when computing the sector offset. */
    > > + sector = (unsigned long long)bh->b_blocknr * (bh->b_size >> 9);
    >
    > Ok so far, looks fine.
    >
    > But what the heck is this:
    >
    > > +#if !defined(CONFIG_LBD) && BITS_PER_LONG == 32
    > > + if (unlikely(sector != (sector_t)sector))
    > > +#else
    > > + if (unlikely(((bh->b_blocknr >> 32) * (bh->b_size >> 9)) >=
    > > + 0xffffffff00000000ULL))
    > > +#endif
    >
    > I don't understand the #ifdef at all.
    >
    > Why isn't that just a
    >
    > if (unlikely(sector != (sector_t)sector))
    >
    > and that's it? What does this have to do with CONFIG_LBD or BITS_PER_LONG,
    > or anything at all?
    >
    > If the sector number fits in a sector_t, we're all good.

    I think you missed that Andrewas said he is worried about 64-bit overflows
    as well. And you would not catch those with the sector !=
    (sector_t)sector test because you would be comparing two 64-bit values
    together so they always match...

    Hence why he shifts the value right by 32 bits then multiplies and tests
    the result for overflowing 32-bits which if it does it means it would
    overflow the 64-bit multiplication, too therefor your "sector" is
    truncated.

    Makes sense to me in a some very convoluted sickening way... (-;

    Best regards,

    Anton
    --
    Anton Altaparmakov <aia21 at cam.ac.uk> (replace at with @)
    Unix Support, Computing Service, University of Cambridge, CB2 3QH, UK
    Linux NTFS maintainer, http://www.linux-ntfs.org/
    -
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

    \
     
     \ /
      Last update: 2006-05-19 00:14    [W:0.026 / U:60.764 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site