From: Anton Altaparmakov <aia21@cam.ac.uk>
To: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Andrew Morton <akpm@linux-foundation.org>,
Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
linux-fsdevel <linux-fsdevel@vger.kernel.org>,
Hugh Dickins <hughd@google.com>, stable <stable@vger.kernel.org>
Subject: Re: [PATCH] Fix nasty 32-bit overflow bug in buffer i/o code.
Date: Mon, 22 Sep 2014 16:46:01 +0100 [thread overview]
Message-ID: <1844C797-F3C7-4EA7-BBE8-520A7609C743@cam.ac.uk> (raw)
In-Reply-To: <CA+55aFwAdsJ9EWTx3QvMxs2ReTX19Xvd_s7wc+KQ8qnJjg0WDA@mail.gmail.com>
Hi Linus,
On 22 Sep 2014, at 16:33, Linus Torvalds <torvalds@linux-foundation.org> wrote:
> On Mon, Sep 22, 2014 at 8:29 AM, Anton Altaparmakov <aia21@cam.ac.uk> wrote:
>>
>> You could do "block & ~(sector_t)(size - 1)" instead of "(sector_t)index << sizebits" if you prefer but not sure that is an improvement!
>
> No, it would be even worse. Something like
>
> block & ~(sector_t)((size >> 9) - 1)
>
> because block is the sector number (ie 512-byte) and size is in bytes.
Oops, sorry. But I think you got it wrong, too as you are ignoring the PAGE_SIZE - as was I but it is what we need to align to in addition to the problem of "size" being in bytes. So I think the correct mask is actually based on sizebits which reflects the number of blocks per page thus:
block & ~(sector_t)((1 << sizebits) - 1)
In any case the shift is the lesser evil I think as it is at least obviously correct whilst getting the right mask has taken us a few iterations of correcting each other! (-:
PS. Thank you for taking my patch and correcting the misleading description!
Best regards,
Anton
> Linus
--
Anton Altaparmakov <aia21 at cam.ac.uk> (replace at with @)
University of Cambridge Information Services, Roger Needham Building
7 JJ Thomson Avenue, Cambridge, CB3 0RB, UK
prev parent reply other threads:[~2014-09-22 15:46 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-09-22 0:53 [PATCH] Fix nasty 32-bit overflow bug in buffer i/o code Anton Altaparmakov
2014-09-22 4:43 ` Hugh Dickins
2014-09-22 9:30 ` Anton Altaparmakov
2014-09-22 10:36 ` Hugh Dickins
2014-09-22 11:01 ` Anton Altaparmakov
2014-09-22 15:18 ` Linus Torvalds
2014-09-22 15:24 ` Linus Torvalds
2014-09-22 15:29 ` Anton Altaparmakov
2014-09-22 15:33 ` Linus Torvalds
2014-09-22 15:46 ` Anton Altaparmakov [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=1844C797-F3C7-4EA7-BBE8-520A7609C743@cam.ac.uk \
--to=aia21@cam.ac.uk \
--cc=akpm@linux-foundation.org \
--cc=hughd@google.com \
--cc=linux-fsdevel@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=stable@vger.kernel.org \
--cc=torvalds@linux-foundation.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).