[PATCH] make kernel ignore bogus partitions

[PATCH] make kernel ignore bogus partitions

[Mike Miller (OS Dev)]

Patch 1/1
Sometimes partitions claim to be larger than the reported capacity of a
disk device. This patch makes the kernel ignore those partitions.

Signed-off-by: Mike Miller <mike.miller@hp.com>
Signed-off-by: Stephen Cameron <steve.cameron@hp.com>

Please consider this for inclusion.


 fs/partitions/check.c |    5 +++++
 1 files changed, 5 insertions(+)

--- linux-2.6.14/fs/partitions/check.c~partition_vs_capacity	2006-01-06 09:32:14.000000000 -0600
+++ linux-2.6.14-root/fs/partitions/check.c	2006-01-06 11:24:50.000000000 -0600
@@ -382,6 +382,11 @@ int rescan_partitions(struct gendisk *di
 		sector_t from = state->parts[p].from;
 		if (!size)
 			continue;
+		if (from+size-1 > get_capacity(disk)) {
+			printk(" %s: p%d exceeds device capacity, ignoring.\n", 
+				disk->disk_name, p);
+			continue;
+		}
 		add_partition(disk, p, from, size);
 #ifdef CONFIG_BLK_DEV_MD
 		if (state->parts[p].flags)

_

Re: [PATCH] make kernel ignore bogus partitions

[Andries Brouwer]

On Wed, May 03, 2006 at 04:00:55PM -0500, Mike Miller (OS Dev) wrote:
> Patch 1/1
> Sometimes partitions claim to be larger than the reported capacity of a
> disk device. This patch makes the kernel ignore those partitions.
> 
> Signed-off-by: Mike Miller <mike.miller@hp.com>
> Signed-off-by: Stephen Cameron <steve.cameron@hp.com>

> +		if (from+size-1 > get_capacity(disk)) {
> +			printk(" %s: p%d exceeds device capacity, ignoring.\n", 
> +				disk->disk_name, p);
> +			continue;
> +		}

I debated for a while with myself whether I should like or dislike
such a patch. On the one hand, this partition stuff is rather messy,
and if you invent strict rules that partitions should satisfy then
during the transition lots of people will be unhappy, but afterwards
the stuff may be less messy.

On the other hand, such changes do indeed make people unhappy.
Indeed, with this change one of my systems does not boot anymore.

There can be reasons, or there can have been reasons, for partitions
larger than the disk. Maybe the disk has a jumper clipping the capacity
while in other machines such a jumper is unnecessary, or while soon
after booting the setmax utility is called to set the disk to full
capacity again.
Or, while doing forensics on a disk one copies the start to some
other disk, and that other disk may be smaller.
Etc.

So, it seems that Linux loses a little bit of its power when such things
are made impossible.

Andries

Re: [PATCH] make kernel ignore bogus partitions

[Douglas Gilbert]

Andries Brouwer wrote:
> On Wed, May 03, 2006 at 04:00:55PM -0500, Mike Miller (OS Dev) wrote:
> 
>>Patch 1/1
>>Sometimes partitions claim to be larger than the reported capacity of a
>>disk device. This patch makes the kernel ignore those partitions.
>>
>>Signed-off-by: Mike Miller <mike.miller@hp.com>
>>Signed-off-by: Stephen Cameron <steve.cameron@hp.com>
> 
> 
>>+		if (from+size-1 > get_capacity(disk)) {
>>+			printk(" %s: p%d exceeds device capacity, ignoring.\n", 
>>+				disk->disk_name, p);
>>+			continue;
>>+		}
> 
> 
> I debated for a while with myself whether I should like or dislike
> such a patch. On the one hand, this partition stuff is rather messy,
> and if you invent strict rules that partitions should satisfy then
> during the transition lots of people will be unhappy, but afterwards
> the stuff may be less messy.
> 
> On the other hand, such changes do indeed make people unhappy.
> Indeed, with this change one of my systems does not boot anymore.
> 
> There can be reasons, or there can have been reasons, for partitions
> larger than the disk. Maybe the disk has a jumper clipping the capacity
> while in other machines such a jumper is unnecessary, or while soon
> after booting the setmax utility is called to set the disk to full
> capacity again.
> Or, while doing forensics on a disk one copies the start to some
> other disk, and that other disk may be smaller.
> Etc.

Andries,
With the creative use of the MODE SELECT SCSI command
one can "short stroke" a disk, making subsequent READ
CAPACITY commands report less than is actually available.
READ and WRITE commands also would be crimped. For
example a 300 GB SCSI disk could be made to report
a capacity of 1 sector. [see sg_format in sg3_utils]

More practically RAID replacement disks may use this
facility if the firmware wants all disks the same
size and a smaller size disk (e.g. 18 GB SCSI disk) is
no longer available.

Without a product manual in which a manufacturer states
what the number of sectors should be, it may not be
obvious short stroking has occurred.


There are other situations I have come across, that can
be made to work if you know what is happening. When I
put a 160 GB PATA disk in an external USB enclosure that
doesn't support 48 bit lba, then I can't access anything
above the 137 (?) GB mark. By arranging my partitions
accordingly (e.g. 3 under, 1 over) the lower partitions
are still useable in the USB enclosure.

> So, it seems that Linux loses a little bit of its power when such things
> are made impossible.

Doug Gilbert

Re: [PATCH] make kernel ignore bogus partitions

[David Greaves]

Andries Brouwer wrote:
> On Wed, May 03, 2006 at 04:00:55PM -0500, Mike Miller (OS Dev) wrote:
>   
>> Patch 1/1
>> Sometimes partitions claim to be larger than the reported capacity of a
>> disk device. This patch makes the kernel ignore those partitions.
>>     
> Or, while doing forensics on a disk one copies the start to some
> other disk, and that other disk may be smaller.
> Etc.
>
> So, it seems that Linux loses a little bit of its power when such things
> are made impossible.
>   
I've had similar situations when trying to recover data from failed devices.
Equally - if you don't know what's going on then partition/disk size
mismatch is a bad thing.
A loud warning may be more appropriate (and useful) than an ignore.

David

Re: [PATCH] make kernel ignore bogus partitions

[Jan Engelhardt]

>>> Sometimes partitions claim to be larger than the reported capacity of a
>>> disk device. This patch makes the kernel ignore those partitions.
>>>     
>> Or, while doing forensics on a disk one copies the start to some
>> other disk, and that other disk may be smaller.
>> Etc.
>>
>> So, it seems that Linux loses a little bit of its power when such things
>> are made impossible.
>>   
>I've had similar situations when trying to recover data from failed devices.
>Equally - if you don't know what's going on then partition/disk size
>mismatch is a bad thing.
>A loud warning may be more appropriate (and useful) than an ignore.

I also consider a warning message more helpful.
OTOH, what if the disk will be automounted at boot? The boot scripts 
won't catch it if it just a warning, whereas if it was ignored, no mount 
would occur.
Maybe a bootparam toggling warning/ignore as the solution?


Jan Engelhardt
-- 

reiser4 bug [was Re: 2.6.17-rc3-mm1]

[Alexander Gran]

[-- Attachment #1: Type: text/plain, Size: 803 bytes --]

Hi all,

2.6.17-rc3-mm1 doesn't get up running  here, it bugs around while init runs:
I cannot login afterwards, and syslog did not get the bug too. So here are 
some poor screenshots from my Treo650 (digicam is broken, sorry..;)
EIP is in clear_inode.
Trace:
reiser4_delete_inode+0x6c/0xd0
d_delete+0xf0/0x10f
reiser4_delete_inode+0x0/0xd0
generic_delete_inode+0x6b/0xfb
input+0x5c/0x68
do_unlikat+0xd7/0x12c
sysenter_past_esp+0x54/0x75
__hidp_send_ctrl_message+0xb4/0xfa
details:
http://zodiac.dnsalias.org/images/1.jpg
http://zodiac.dnsalias.org/images/2.jpg
http://zodiac.dnsalias.org/images/3.jpg
http://zodiac.dnsalias.org/images/4.jpg
Kernel config:
http://zodiac.dnsalias.org/images/config
System is my T40p, as usual. running an up2date debian unstable.

regards
Alex

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]

Re: reiser4 bug [was Re: 2.6.17-rc3-mm1]

[Joe Feise]

Try the patch from here: 
http://marc.theaimsgroup.com/?l=reiserfs&m=114709188305181&w=2
That helped me get past the bootup phase (currently 8 hours uptime). 

 -Joe 

Alexander Gran writes: 

> Hi all, 
> 
> 2.6.17-rc3-mm1 doesn't get up running  here, it bugs around while init runs:
> I cannot login afterwards, and syslog did not get the bug too. So here are 
> some poor screenshots from my Treo650 (digicam is broken, sorry..;)
> EIP is in clear_inode.
> Trace:
> reiser4_delete_inode+0x6c/0xd0
> d_delete+0xf0/0x10f
> reiser4_delete_inode+0x0/0xd0
> generic_delete_inode+0x6b/0xfb
> input+0x5c/0x68
> do_unlikat+0xd7/0x12c
> sysenter_past_esp+0x54/0x75
> __hidp_send_ctrl_message+0xb4/0xfa
> details:
> http://zodiac.dnsalias.org/images/1.jpg
> http://zodiac.dnsalias.org/images/2.jpg
> http://zodiac.dnsalias.org/images/3.jpg
> http://zodiac.dnsalias.org/images/4.jpg
> Kernel config:
> http://zodiac.dnsalias.org/images/config
> System is my T40p, as usual. running an up2date debian unstable. 
> 
> regards
> Alex

Re: reiser4 bug [was Re: 2.6.17-rc3-mm1]

[Alexander Gran]

[-- Attachment #1: Type: text/plain, Size: 1311 bytes --]

Nope, did not work...
regards
Alex

Am Dienstag, 9. Mai 2006 01:21 schrieb Joe Feise:
> Try the patch from here:
> http://marc.theaimsgroup.com/?l=reiserfs&m=114709188305181&w=2
> That helped me get past the bootup phase (currently 8 hours uptime).
>
>  -Joe
>
> Alexander Gran writes:
> > Hi all,
> >
> > 2.6.17-rc3-mm1 doesn't get up running  here, it bugs around while init
> > runs: I cannot login afterwards, and syslog did not get the bug too. So
> > here are some poor screenshots from my Treo650 (digicam is broken,
> > sorry..;) EIP is in clear_inode.
> > Trace:
> > reiser4_delete_inode+0x6c/0xd0
> > d_delete+0xf0/0x10f
> > reiser4_delete_inode+0x0/0xd0
> > generic_delete_inode+0x6b/0xfb
> > input+0x5c/0x68
> > do_unlikat+0xd7/0x12c
> > sysenter_past_esp+0x54/0x75
> > __hidp_send_ctrl_message+0xb4/0xfa
> > details:
> > http://zodiac.dnsalias.org/images/1.jpg
> > http://zodiac.dnsalias.org/images/2.jpg
> > http://zodiac.dnsalias.org/images/3.jpg
> > http://zodiac.dnsalias.org/images/4.jpg
> > Kernel config:
> > http://zodiac.dnsalias.org/images/config
> > System is my T40p, as usual. running an up2date debian unstable.
> >
> > regards
> > Alex

-- 
Encrypted Mails welcome.
PGP-Key at http://zodiac.dnsalias.org/misc/pgpkey.asc | Key-ID: 0x6D7DD291

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]

Re: [PATCH] make kernel ignore bogus partitions

[Andrew Morton]

"Mike Miller (OS Dev)" <mikem@beardog.cca.cpqcorp.net> wrote:
>
> Patch 1/1
> Sometimes partitions claim to be larger than the reported capacity of a
> disk device. This patch makes the kernel ignore those partitions.
> 
> Signed-off-by: Mike Miller <mike.miller@hp.com>
> Signed-off-by: Stephen Cameron <steve.cameron@hp.com>
> 
> Please consider this for inclusion.
> 
> 
>  fs/partitions/check.c |    5 +++++
>  1 files changed, 5 insertions(+)
> 
> --- linux-2.6.14/fs/partitions/check.c~partition_vs_capacity	2006-01-06 09:32:14.000000000 -0600
> +++ linux-2.6.14-root/fs/partitions/check.c	2006-01-06 11:24:50.000000000 -0600
> @@ -382,6 +382,11 @@ int rescan_partitions(struct gendisk *di
>  		sector_t from = state->parts[p].from;
>  		if (!size)
>  			continue;
> +		if (from+size-1 > get_capacity(disk)) {
> +			printk(" %s: p%d exceeds device capacity, ignoring.\n", 
> +				disk->disk_name, p);
> +			continue;
> +		}
>  		add_partition(disk, p, from, size);
>  #ifdef CONFIG_BLK_DEV_MD
>  		if (state->parts[p].flags)

Shouldn't that be

	if (from+size > get_capacity(disk)) {

?

Re: [PATCH] make kernel ignore bogus partitions

[Andries Brouwer]

On Tue, May 09, 2006 at 12:41:38PM -0700, Andrew Morton wrote:

> > +		if (from+size-1 > get_capacity(disk)) {
> > +			printk(" %s: p%d exceeds device capacity, ignoring.\n", 
> > +				disk->disk_name, p);
> > +			continue;
> > +		}
> >  		add_partition(disk, p, from, size);

> Shouldn't that be
> 
> 	if (from+size > get_capacity(disk)) {
> 
> ?

Ha, you are awake. Yes, it should.
And no "ignoring". And no "continue". E.g.:

	printk(" %s: warning: p%d exceeds device capacity.\n", ...);

Andries

Re: [PATCH] make kernel ignore bogus partitions

[Andrew Morton]

Andries Brouwer <Andries.Brouwer@cwi.nl> wrote:
>
> On Tue, May 09, 2006 at 12:41:38PM -0700, Andrew Morton wrote:
> 
> > > +		if (from+size-1 > get_capacity(disk)) {
> > > +			printk(" %s: p%d exceeds device capacity, ignoring.\n", 
> > > +				disk->disk_name, p);
> > > +			continue;
> > > +		}
> > >  		add_partition(disk, p, from, size);
> 
> > Shouldn't that be
> > 
> > 	if (from+size > get_capacity(disk)) {
> > 
> > ?
> 
> Ha, you are awake.

Opinions differ.

> Yes, it should.
> And no "ignoring". And no "continue". E.g.:
> 
> 	printk(" %s: warning: p%d exceeds device capacity.\n", ...);
> 

So you're saying that after detecting this inconsistency we should proceed
to use the partition anyway?

For what reason?

Re: [PATCH] make kernel ignore bogus partitions

[Andries Brouwer]

On Thu, May 11, 2006 at 04:00:14AM -0700, Andrew Morton wrote:

> > Yes, it should.
> > And no "ignoring". And no "continue". E.g.:
> > 
> > 	printk(" %s: warning: p%d exceeds device capacity.\n", ...);
> > 
> 
> So you're saying that after detecting this inconsistency we should proceed
> to use the partition anyway?
> 
> For what reason?

The normal situation is that partitions are contained within
the disk. In the normal situation the test is superfluous.

Suppose the test fails. Why might that be? There isn't really
a good scenario where this is a mistake. In all the (rare) cases
that I can imagine, it would make matters worse to reject the
partition and make access impossible (or at least more difficult).

Case 1: The kernel is mistaken about the size of the disk.
(There are commands to clip a disk to a certain capacity,
there are jumpers to tell a disk that it should report a certain
capacity etc. Usually this is because of BIOS bugs. In bad cases
the machine will crash in the BIOS and hence fail to boot if
the disk reports full capacity.)
In such cases actually accessing the blocks of the partition
may work fine, or may work fine after running an unclip utility.
I wrote "setmax" some years ago precisely for this reason.

Case 2: There was a messy partition table (maybe just a rounding
error) but the actual filesystem on the partition is contained
in the physical disk. Now using the filesystem goes without problem.

Case 3: Both partition and filesystem extend beyond the end of the disk.
In forensic or debugging situations one often uses a copy of the start
of a disk. Now access beyond the end gives an expected I/O error.

Andries

Re: [PATCH] make kernel ignore bogus partitions

[Mike Miller (OS Dev)]

On Thu, May 11, 2006 at 01:51:17PM +0200, Andries Brouwer wrote:
> On Thu, May 11, 2006 at 04:00:14AM -0700, Andrew Morton wrote:
> 
> > > Yes, it should.
> > > And no "ignoring". And no "continue". E.g.:
> > > 
> > > 	printk(" %s: warning: p%d exceeds device capacity.\n", ...);
> > > 
> > 
> > So you're saying that after detecting this inconsistency we should proceed
> > to use the partition anyway?
> > 
> > For what reason?
> 
> The normal situation is that partitions are contained within
> the disk. In the normal situation the test is superfluous.
> 
> Suppose the test fails. Why might that be? There isn't really
> a good scenario where this is a mistake. In all the (rare) cases
> that I can imagine, it would make matters worse to reject the
> partition and make access impossible (or at least more difficult).
> 
> Case 1: The kernel is mistaken about the size of the disk.
> (There are commands to clip a disk to a certain capacity,
> there are jumpers to tell a disk that it should report a certain
> capacity etc. Usually this is because of BIOS bugs. In bad cases
> the machine will crash in the BIOS and hence fail to boot if
> the disk reports full capacity.)
> In such cases actually accessing the blocks of the partition
> may work fine, or may work fine after running an unclip utility.
> I wrote "setmax" some years ago precisely for this reason.
> 
> Case 2: There was a messy partition table (maybe just a rounding
> error) but the actual filesystem on the partition is contained
> in the physical disk. Now using the filesystem goes without problem.
> 
> Case 3: Both partition and filesystem extend beyond the end of the disk.
> In forensic or debugging situations one often uses a copy of the start
> of a disk. Now access beyond the end gives an expected I/O error.


Continuing to use the partition will result in io errors when accessing 
any block that is past the end of the physical device. We see this as
unacceptable. Why would it make matters worse to prevent access to the
partition? The partition _should_ be correct.

mikem

Re: [PATCH] make kernel ignore bogus partitions

[Daniel Barkalow]

On Thu, 11 May 2006, Andries Brouwer wrote:

> The normal situation is that partitions are contained within
> the disk. In the normal situation the test is superfluous.
> 
> Suppose the test fails. Why might that be? There isn't really
> a good scenario where this is a mistake. In all the (rare) cases
> that I can imagine, it would make matters worse to reject the
> partition and make access impossible (or at least more difficult).
> 
> Case 1: The kernel is mistaken about the size of the disk.
> (There are commands to clip a disk to a certain capacity,
> there are jumpers to tell a disk that it should report a certain
> capacity etc. Usually this is because of BIOS bugs. In bad cases
> the machine will crash in the BIOS and hence fail to boot if
> the disk reports full capacity.)
> In such cases actually accessing the blocks of the partition
> may work fine, or may work fine after running an unclip utility.
> I wrote "setmax" some years ago precisely for this reason.

Perhaps the kernel should try reading beyond the ends of disks when it 
detects them, so that it can determine if there's actually available 
storage there, and automatically increase the size if there is? Or, at 
least, it could check whether the medium actually goes out to the point 
the partition table implies, and suppress the I/O error if the disk 
actually ends where it claims to.

> Case 2: There was a messy partition table (maybe just a rounding
> error) but the actual filesystem on the partition is contained
> in the physical disk. Now using the filesystem goes without problem.

I think I've seen cameras format SD cards like this. If I understand the 
situation correctly, it's a pain to mount them, because the kernel pokes 
around beyond the end of the medium trying to determine the filesystem 
type. In this case, wouldn't the right thing be to add the partition as 
ending at the end of the disk, rather than where it claims to?

> Case 3: Both partition and filesystem extend beyond the end of the disk.
> In forensic or debugging situations one often uses a copy of the start
> of a disk. Now access beyond the end gives an expected I/O error.

I think cropping the partition to the size of the copied area is fine 
here, too, and should generate I/O errors on the partition without errors 
on the underlying block device, so it will be more clear that the hardware 
is fine, but your filesystem has problems (i.e., part of it isn't 
included).

In any case, I don't think it makes sense to leave the partition and 
underlying device inconsistant, rather than correcting one or the other.

	-Daniel
*This .sig left intentionally blank*

Re: [PATCH] make kernel ignore bogus partitions

[Jan Engelhardt]

>
>Perhaps the kernel should try reading beyond the ends of disks when it 
>detects them, so that it can determine if there's actually available 
>storage there, and automatically increase the size if there is? Or, at 
>least, it could check whether the medium actually goes out to the point 
>the partition table implies, and suppress the I/O error if the disk 
>actually ends where it claims to.
>
Sounds like a good idea. In fact, I had miscreated a sun disklabel on a 
disk because it has a slightly different notion of cylinders that I am used 
to from x86; IOW:

dmesg:
SCSI device sdb: 35378533 512-byte hdwr sectors (18114 MB)

fdisk:
Disk /dev/sdb (Sun disk label): 19 heads, 248 sectors, 7200 rpm
7508 cylinders, 2 alternate cylinders, 7510 physical cylinders
0 extra sects/cyl, interleave 1:1
(should have been 7506 cyl, 2 alt, 7508 phys)

And Solaris rightfully barfs about it when scanning disks,
because 7510*19*248 > 35378533. Linux keeps silent,
and I am not sure if I have a silent data corruption there (currently not 
as it seems).
(Since it's just a test box ATM, it's not critical.)


Jan Engelhardt
-- 

Re: [PATCH] make kernel ignore bogus partitions

[Phillip Susi]

It looks like this patch got merged in to only warn about partitions 
going beyond the end of the device.  What still concerns me is that I ( 
and others ) get a lot of IO error kernel messages during boot because 
we boot from a raid0 and the first disk in the set appears to contain a 
valid partition table that lists partitions larger than the single disk 
( since the partitions span both disks ).  This causes the kernel to 
complain when it probes the partitions as it tries to read beyond the 
end of the device.

The arguments in this thread for not discarding such partitions out of 
hand make sense to me, but I wonder: why does the kernel complain about 
IO errors to the disk when it KNOWS it is making an invalid request ( to 
sectors beyond the end of the device )?  Attempting the IO anyhow makes 
sense in a way if sometimes the kernel can detect the size wrongly, but 
if the IO fails, maybe the error message should be suppressed if it is 
beyond the detected end of device?


Jan Engelhardt wrote:
>> Perhaps the kernel should try reading beyond the ends of disks when it 
>> detects them, so that it can determine if there's actually available 
>> storage there, and automatically increase the size if there is? Or, at 
>> least, it could check whether the medium actually goes out to the point 
>> the partition table implies, and suppress the I/O error if the disk 
>> actually ends where it claims to.
>>
> Sounds like a good idea. In fact, I had miscreated a sun disklabel on a 
> disk because it has a slightly different notion of cylinders that I am used 
> to from x86; IOW:
> 
> dmesg:
> SCSI device sdb: 35378533 512-byte hdwr sectors (18114 MB)
> 
> fdisk:
> Disk /dev/sdb (Sun disk label): 19 heads, 248 sectors, 7200 rpm
> 7508 cylinders, 2 alternate cylinders, 7510 physical cylinders
> 0 extra sects/cyl, interleave 1:1
> (should have been 7506 cyl, 2 alt, 7508 phys)
> 
> And Solaris rightfully barfs about it when scanning disks,
> because 7510*19*248 > 35378533. Linux keeps silent,
> and I am not sure if I have a silent data corruption there (currently not 
> as it seems).
> (Since it's just a test box ATM, it's not critical.)
> 
> 
> Jan Engelhardt

Re: [PATCH] make kernel ignore bogus partitions

[Christian Schmidt]

Phillip Susi wrote:
> It looks like this patch got merged in to only warn about partitions
> going beyond the end of the device.  What still concerns me is that I (
> and others ) get a lot of IO error kernel messages during boot because
> we boot from a raid0 and the first disk in the set appears to contain a
> valid partition table that lists partitions larger than the single disk
> ( since the partitions span both disks ).  This causes the kernel to
> complain when it probes the partitions as it tries to read beyond the
> end of the device.
> 
> The arguments in this thread for not discarding such partitions out of
> hand make sense to me, but I wonder: why does the kernel complain about
> IO errors to the disk when it KNOWS it is making an invalid request ( to
> sectors beyond the end of the device )?  Attempting the IO anyhow makes
> sense in a way if sometimes the kernel can detect the size wrongly, but
> if the IO fails, maybe the error message should be suppressed if it is
> beyond the detected end of device?

I wonder if this at some time can be abused by someone plugging in a
purposely mispartitioned device into a machine. The software RAID (raid
0 at least; others probably) drivers at least can be driven into null
pointer dereferences this way; see my earlier mail to the list for
details. In summary: The block layer allows a request beyond the end to
come through to the software RAID driver, which doesn't check if the
access is beyond the end of its device and tries to read some data
structures which don't span that far.

> Jan Engelhardt wrote:
>>> Perhaps the kernel should try reading beyond the ends of disks when
>>> it detects them, so that it can determine if there's actually
>>> available storage there, and automatically increase the size if there
>>> is? Or, at least, it could check whether the medium actually goes out
>>> to the point the partition table implies, and suppress the I/O error
>>> if the disk actually ends where it claims to.
>>>
>> Sounds like a good idea. In fact, I had miscreated a sun disklabel on
>> a disk because it has a slightly different notion of cylinders that I
>> am used to from x86; IOW:
>>
>> dmesg:
>> SCSI device sdb: 35378533 512-byte hdwr sectors (18114 MB)
>>
>> fdisk:
>> Disk /dev/sdb (Sun disk label): 19 heads, 248 sectors, 7200 rpm
>> 7508 cylinders, 2 alternate cylinders, 7510 physical cylinders
>> 0 extra sects/cyl, interleave 1:1
>> (should have been 7506 cyl, 2 alt, 7508 phys)
>>
>> And Solaris rightfully barfs about it when scanning disks,
>> because 7510*19*248 > 35378533. Linux keeps silent,
>> and I am not sure if I have a silent data corruption there (currently
>> not as it seems).
>> (Since it's just a test box ATM, it's not critical.)
>>
>>
>> Jan Engelhardt
> 
> -
> 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/

Re: [PATCH] make kernel ignore bogus partitions

[Mike Miller (OS Dev)]

On Thu, May 11, 2006 at 04:00:14AM -0700, Andrew Morton wrote:
> Andries Brouwer <Andries.Brouwer@cwi.nl> wrote:
> >
> > On Tue, May 09, 2006 at 12:41:38PM -0700, Andrew Morton wrote:
> > 
> > > > +		if (from+size-1 > get_capacity(disk)) {
> > > > +			printk(" %s: p%d exceeds device capacity, ignoring.\n", 
> > > > +				disk->disk_name, p);
> > > > +			continue;
> > > > +		}
> > > >  		add_partition(disk, p, from, size);
> > 
> > > Shouldn't that be
> > > 
> > > 	if (from+size > get_capacity(disk)) {
> > > 
> > > ?
> > 
> > Ha, you are awake.
> 
> Opinions differ.
> 
> > Yes, it should.
> > And no "ignoring". And no "continue". E.g.:
> > 
> > 	printk(" %s: warning: p%d exceeds device capacity.\n", ...);
> > 
> 
> So you're saying that after detecting this inconsistency we should proceed
> to use the partition anyway?
> 
> For what reason?

Using the partition will result in io errors when accessing beyond the end
of the device. Most users don't appreciate that. It makes them nervous
about the hw.

mikem

Re: [PATCH] make kernel ignore bogus partitions

[Mike Miller (OS Dev)]

On Tue, May 09, 2006 at 12:41:38PM -0700, Andrew Morton wrote:
> "Mike Miller (OS Dev)" <mikem@beardog.cca.cpqcorp.net> wrote:
> >
> > Patch 1/1
> > Sometimes partitions claim to be larger than the reported capacity of a
> > disk device. This patch makes the kernel ignore those partitions.
> > 
> > Signed-off-by: Mike Miller <mike.miller@hp.com>
> > Signed-off-by: Stephen Cameron <steve.cameron@hp.com>
> > 
> > Please consider this for inclusion.
> > 
> > 
> >  fs/partitions/check.c |    5 +++++
> >  1 files changed, 5 insertions(+)
> > 
> > --- linux-2.6.14/fs/partitions/check.c~partition_vs_capacity	2006-01-06 09:32:14.000000000 -0600
> > +++ linux-2.6.14-root/fs/partitions/check.c	2006-01-06 11:24:50.000000000 -0600
> > @@ -382,6 +382,11 @@ int rescan_partitions(struct gendisk *di
> >  		sector_t from = state->parts[p].from;
> >  		if (!size)
> >  			continue;
> > +		if (from+size-1 > get_capacity(disk)) {
> > +			printk(" %s: p%d exceeds device capacity, ignoring.\n", 
> > +				disk->disk_name, p);
> > +			continue;
> > +		}
> >  		add_partition(disk, p, from, size);
> >  #ifdef CONFIG_BLK_DEV_MD
> >  		if (state->parts[p].flags)
> 
> Shouldn't that be
> 
> 	if (from+size > get_capacity(disk)) {
> 
> ?
> 
Since the partition size is 0-based this is correct:

	if (from+size-1 > get_capacity(disk)) {

mikem

Re: [PATCH] make kernel ignore bogus partitions

[Andrew Morton]

"Mike Miller (OS Dev)" <mikem@beardog.cca.cpqcorp.net> wrote:
>
> On Tue, May 09, 2006 at 12:41:38PM -0700, Andrew Morton wrote:
> > "Mike Miller (OS Dev)" <mikem@beardog.cca.cpqcorp.net> wrote:
> > >
> > > Patch 1/1
> > > Sometimes partitions claim to be larger than the reported capacity of a
> > > disk device. This patch makes the kernel ignore those partitions.
> > > 
> > > Signed-off-by: Mike Miller <mike.miller@hp.com>
> > > Signed-off-by: Stephen Cameron <steve.cameron@hp.com>
> > > 
> > > Please consider this for inclusion.
> > > 
> > > 
> > >  fs/partitions/check.c |    5 +++++
> > >  1 files changed, 5 insertions(+)
> > > 
> > > --- linux-2.6.14/fs/partitions/check.c~partition_vs_capacity	2006-01-06 09:32:14.000000000 -0600
> > > +++ linux-2.6.14-root/fs/partitions/check.c	2006-01-06 11:24:50.000000000 -0600
> > > @@ -382,6 +382,11 @@ int rescan_partitions(struct gendisk *di
> > >  		sector_t from = state->parts[p].from;
> > >  		if (!size)
> > >  			continue;
> > > +		if (from+size-1 > get_capacity(disk)) {
> > > +			printk(" %s: p%d exceeds device capacity, ignoring.\n", 
> > > +				disk->disk_name, p);
> > > +			continue;
> > > +		}
> > >  		add_partition(disk, p, from, size);
> > >  #ifdef CONFIG_BLK_DEV_MD
> > >  		if (state->parts[p].flags)
> > 
> > Shouldn't that be
> > 
> > 	if (from+size > get_capacity(disk)) {
> > 
> > ?
> > 
> Since the partition size is 0-based this is correct:
> 
> 	if (from+size-1 > get_capacity(disk)) {
> 

Don't think so.

If `from' is 0 and `size' is 1025 and get_capacity() is 1024 then we have

	if (1024 > 1024)

which returns false.

Re: [PATCH] make kernel ignore bogus partitions

[Alan Cox]

On Iau, 2006-05-11 at 09:27 -0700, Andrew Morton wrote:
> "Mike Miller (OS Dev)" <mikem@beardog.cca.cpqcorp.net> wrote:
> >
> > On Tue, May 09, 2006 at 12:41:38PM -0700, Andrew Morton wrote:
> > > "Mike Miller (OS Dev)" <mikem@beardog.cca.cpqcorp.net> wrote:
> > > >
> > > > Patch 1/1
> > > > Sometimes partitions claim to be larger than the reported capacity of a
> > > > disk device. This patch makes the kernel ignore those partitions.

The problem with ignoring such partitions is that you will then get
burned on some PC setups and also that existing partitions may move
number on some partitioning schemes.

Allocating them but setting the reported size to zero would cure the
latter problem, but I'm not sure what the right thing to do is about
extended partition tables that look like this

0 Partition Table
  Bootblock
  ...
  Extended Partition to disk end
  Partition
HPA-------------------------- (reported disk size)
  Suspend partition
  BIOS bits
disk end -----

ie /dev/hda5 might be valid but not /dev/hda4 which contains it...