mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Andreas Dilger <adilger@clusterfs.com>
To: Andrew Morton <akpm@osdl.org>
Cc: "Stephen C. Tweedie" <sct@redhat.com>,
	torvalds@osdl.org, aia21@cam.ac.uk,
	ext2-devel@lists.sourceforge.net, linux-kernel@vger.kernel.org
Subject: Re: [PATCH] sector_t overflow in block layer
Date: Fri, 19 May 2006 14:55:51 -0600	[thread overview]
Message-ID: <20060519205550.GI5964@schatzie.adilger.int> (raw)
In-Reply-To: <20060519131130.71c390d9.akpm@osdl.org>

On May 19, 2006  13:11 -0700, Andrew Morton wrote:
> btw, it seems odd to me that we're trying to handle the
> device-too-large-for-sector_t problem at the submit_bh() level.  What
> happens if someone uses submit_bio()?  Isn't it something we can check at
> mount time, or partition parsing, or...?

Doing so at mount time will be added to ext3 shortly.  That said, I'd still
like a check in the block layer to avoid the silent corruption possible if
the filesystem makes a mistake, or for those filesystems that haven't added
such a check yet, or new filesystems that don't in the future.

I saw that xfs and ocfs2 have some checks at fill_super time related
to sector_t and CONFIG_LBD, which I found AFTER I saw that ext3 already
had a problem and was searching for where CONFIG_LBD was used. Looking
at that code, however, it isn't even clear there that this is checking
for sector_t overflow, only what the maximum file size is, so they may
also be susceptible to the same corruption.

As for submit_bio() I don't think it is even POSSIBLE to check for overflow
at that stage because the bi_sector has already been truncated to sector_t.
It would have to be done in the caller of submit_bio(), as in submit_bh().

As for the kernel completely disallowing access to a block device larger
than 2TB when CONFIG_LBD isn't set is a separate question entirely.
Stephen suggested (and I can believe this happening, just did it myself
last week during testing on an 8TB+delta device) that a filesystem may be
formatted to only use the accessible part of the device.  Having the block
layer fail IO beyond the limit is good, not being able to use the device
because it is just a bit bigger than the safe limit is not.

One extra suggestion that might be safe and acceptible all around is if
a device is larger than 2TB w/o a 64-bit sector_t that the block device
size itself be truncated in the kernel to 2TB-512.  This at least prevents
userspace tools from trying to e.g. format a 3TB filesystem on a device
that will just corrupt the filesystem.

I don't think partitions on a larger device make any difference, since
the kernel will remap them to the final sector offset internally in the
end, and still overflow sector_t later (which could itself be checked
also).  I don't think this is sufficient to avoid the submit_bh() check
though, since it doesn't address wrapping when calculating the sector.

Cheers, Andreas
--
Andreas Dilger
Principal Software Engineer
Cluster File Systems, Inc.


  reply	other threads:[~2006-05-19 20:55 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <1147849273.16827.27.camel@localhost.localdomain>
     [not found] ` <m3odxxukcp.fsf@bzzz.home.net>
     [not found]   ` <1147884610.16827.44.camel@localhost.localdomain>
     [not found]     ` <m34pzo36d4.fsf@bzzz.home.net>
     [not found]       ` <1147888715.12067.38.camel@dyn9047017100.beaverton.ibm.com>
     [not found]         ` <m364k4zfor.fsf@bzzz.home.net>
     [not found]           ` <20060517235804.GA5731@schatzie.adilger.int>
     [not found]             ` <1147947803.5464.19.camel@sisko.sctweedie.blueyonder.co.uk>
2006-05-18 18:59               ` Andreas Dilger
2006-05-18 20:58                 ` Alexey Dobriyan
2006-05-18 21:06                 ` Linus Torvalds
2006-05-18 22:12                   ` Anton Altaparmakov
2006-05-18 22:41                     ` Linus Torvalds
2006-05-18 23:23                       ` Andreas Dilger
2006-05-19 19:36                         ` Stephen C. Tweedie
2006-05-19 20:11                           ` Andrew Morton
2006-05-19 20:55                             ` Andreas Dilger [this message]
2006-05-19 21:15                               ` Andrew Morton
2006-05-19 21:05                             ` Stephen C. Tweedie
2006-05-18 22:43                     ` Andreas Dilger
2006-05-22  0:14                     ` Pavel Machek

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=20060519205550.GI5964@schatzie.adilger.int \
    --to=adilger@clusterfs.com \
    --cc=aia21@cam.ac.uk \
    --cc=akpm@osdl.org \
    --cc=ext2-devel@lists.sourceforge.net \
    --cc=linux-kernel@vger.kernel.org \
    --cc=sct@redhat.com \
    --cc=torvalds@osdl.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

all inboxes | Powered by JetHome®