2.6.12-rc3 fails to read partition table

2.6.12-rc3 fails to read partition table

[Hubert Tonneau]

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

2.6.11 and 2.6.11.7 work fine.
2.6.12-rc1 2.6.12-rc2 and 2.6.12-rc3 fail to read partiton table on my laptop,
also 2.6.12-rc3 works fine on another box also running FullPliant.

The meaningfull part of the boot sequence log is:

With 2.6.11.x:
hda: 117210240 sectors (60011 MB) w/8192KiB Cache, CHS=65535/16/63, UDMA(100)
hda: cache flushes supported
 hda: hda1

With 2.6.12-rcx:
hda: 117210240 sectors (60011 MB) w/8192KiB Cache, CHS=65535/16/63, UDMA(100)
hda: cache flushes supported
 hda:
so with 2.6.12-rcx, the boot process ends with an unpleasant:
VFS: Cannot open root device "301" or unknown-bloc(3,1)

The partition table has been created by Pliant partition tool:
http://fullpliant.org/pliant/linux/storage/partition.pli
function 'partition_create' at line 52.
Since most of you are probably not awared of Pliant programming language,
neither FullPliant operating system,
I've attached 'dd if=/dev/hda of=/tmp/partition.bin bs=512 count=1'

[-- Attachment #2: partition.bin --]
[-- Type: application/octet-stream, Size: 512 bytes --]

Re: 2.6.12-rc3 fails to read partition table

[Andries Brouwer]

On Sun, Apr 24, 2005 at 02:37:54PM +0000, Hubert Tonneau wrote:

> 2.6.11 and 2.6.11.7 work fine.
> 2.6.12-rc1 2.6.12-rc2 and 2.6.12-rc3 fail to read partiton table on my laptop,
> also 2.6.12-rc3 works fine on another box also running FullPliant.
> 
> The partition table has been created by Pliant partition tool:
> http://fullpliant.org/pliant/linux/storage/partition.pli
> function 'partition_create' at line 52.

[Heve been away for a while, sorry for a slow reaction]

Yes. I see you create partitions using

ph type := (shunt fs="ext2" or fs="xfs" 83h fs="swap" 82h fs="raid" 0FDh 0)

and it looks like the final 0 is the culprit here.
Earlier Linux disregarded partition types, today 0 means "unused".

Andries

Re: 2.6.12-rc3 fails to read partition table

[Andrew Morton]

Andries Brouwer <aebr@win.tue.nl> wrote:
>
> Earlier Linux disregarded partition types, today 0 means "unused".

A number of people are being bitten by this.  Don't you think we should
revert this change?  

Re: 2.6.12-rc3 fails to read partition table

[Andries Brouwer]

On Thu, May 05, 2005 at 07:43:42PM -0700, Andrew Morton wrote:
> Andries Brouwer <aebr@win.tue.nl> wrote:
> >
> > Earlier Linux disregarded partition types, today 0 means "unused".
> 
> A number of people are being bitten by this.  Don't you think we should
> revert this change?

Executive summary: No strong opinion. Maybe, yes.

Two people report that it fixes an oops, four people notice that
partitions of type 0 are ignored now.

Discussion:

(i) Why was it added?
Because of the report by Uwe Bonnes:

----
the partition table of the USB stick in question is valid:

 1B0:  00 00 00 00 00 00 00 00   53 3F 3C B9 00 00 00 01 ........S?<.....
 1C0:  01 00 06 10 21 7D 25 00   00 00 DB F3 01 00 00 00 ....!}%.........
 1D0:  00 00 00 00 00 00 00 00   00 00 00 00 00 00 00 00 ................
   *
 1F0:  00 00 00 00 00 00 00 54   72 75 6D 70 4D 53 55 AA .......TrumpMSU.

Entry 1 is a FAT partition of exactly the size of the stick, and entries 2
to 4 are empty, marked by id zero. However the manufacturer decided to put a
name string  "Trump" ( /sbin/lsusb gives
Bus 004 Device 012: ID 090a:1bc0 Trumpion Microelectronics, Inc.) just before
the "55 AA" partition table magic and our code reads this string as a
(bogus) size for the fourth entry, taking it for real.

> on a Suse 9.2 System with Suse Hotplug, the phantom partition was somehow
> recognized as Reiserfs, and then the Hotplug mechanism trying to mount the 
> bogus partition as a Reiser Filesystem ended in an Oops...
----

So: the reason to add it is weak: a bad manufacturer sells devices
with a non-standard partition table that happens to work under Windows
because type 0 is ignored there, and a bad Linux vendor does recognition,
also known as guessing, and guesses wrong, and a bad kernel does not
survive mounting garbage as reiserfs.


If the patch is reverted, and we spend some more time fixing
filesystem code so that it survives mounting garbage, then
the kernel is OK and we can put the remaining blame with SuSE.


(ii) Was adding this check a step in the right direction?

No, it was just a bandaid - although handling things more like
other OSs do might be viewed a step in the right direction.

The right direction as far as I am concerned is moving partition parsing
out to user space, to udev or upart or so.


(iii) Should it be reverted?

I have no strong opinion on this.
As far as I know type 0 is ignored by Microsoft and is not
created by Linux vendor installation programs, or by Linux
*fdisk type programs, unless the user asks for it explicitly.
So, the number of people that hit this will be small, and since
they put the 0 there themselves will be able to put something else there.

I think I saw four people so far. Now that all readers of l-k know,
anybody who encounters a problem will be told quickly how to avoid it.


(iv) If I were maintainer of 2.6 - would I revert it?

Hmm... Not sure... Maybe, yes.

Andries

Re: 2.6.12-rc3 fails to read partition table

[Andrew Morton]

Andries Brouwer <Andries.Brouwer@cwi.nl> wrote:
>
> Discussion:

Thanks for that.

> 
> (iv) If I were maintainer of 2.6 - would I revert it?
> 
> Hmm... Not sure... Maybe, yes.
> 

How about the old fallback?


From: Andrew Morton <akpm@osdl.org>

Since early March we've been skipping partitions which have a signature byte
of zero - this was to accomodate an incorrectly manufactured USB stick.

But it broke a few people's setups because it caused device renumbering.

So add a new boot option `msdos_skip_null_part' to enable this workaround.

Signed-off-by: Andrew Morton <akpm@osdl.org>
---

 Documentation/kernel-parameters.txt |    6 ++++++
 fs/partitions/msdos.c               |   14 ++++++++++++--
 2 files changed, 18 insertions(+), 2 deletions(-)

diff -puN fs/partitions/msdos.c~msdos-partitions-null-handling-boot-option fs/partitions/msdos.c
--- 25/fs/partitions/msdos.c~msdos-partitions-null-handling-boot-option	2005-05-06 04:37:32.000000000 -0700
+++ 25-akpm/fs/partitions/msdos.c	2005-05-06 04:46:16.000000000 -0700
@@ -20,6 +20,7 @@
  */
 
 #include <linux/config.h>
+#include <linux/init.h>
 
 #include "check.h"
 #include "msdos.h"
@@ -53,6 +54,15 @@ static inline int is_extended_partition(
 #define MSDOS_LABEL_MAGIC1	0x55
 #define MSDOS_LABEL_MAGIC2	0xAA
 
+static int skip_null_part;
+
+static int __init msdos_skip_null_part(char *str)
+{
+	skip_null_part = 1;
+	return 0;
+}
+__setup("msdos_skip_null_part", msdos_skip_null_part);
+
 static inline int
 msdos_magic_present(unsigned char *p)
 {
@@ -115,7 +125,7 @@ parse_extended(struct parsed_partitions 
 		for (i=0; i<4; i++, p++) {
 			u32 offs, size, next;
 
-			if (SYS_IND(p) == 0)
+			if (skip_null_part && SYS_IND(p) == 0)
 				continue;
 			if (!NR_SECTS(p) || is_extended_partition(p))
 				continue;
@@ -433,7 +443,7 @@ int msdos_partition(struct parsed_partit
 	for (slot = 1 ; slot <= 4 ; slot++, p++) {
 		u32 start = START_SECT(p)*sector_size;
 		u32 size = NR_SECTS(p)*sector_size;
-		if (SYS_IND(p) == 0)
+		if (skip_null_part && SYS_IND(p) == 0)
 			continue;
 		if (!size)
 			continue;
diff -puN Documentation/kernel-parameters.txt~msdos-partitions-null-handling-boot-option Documentation/kernel-parameters.txt
--- 25/Documentation/kernel-parameters.txt~msdos-partitions-null-handling-boot-option	2005-05-06 04:43:52.000000000 -0700
+++ 25-akpm/Documentation/kernel-parameters.txt	2005-05-06 04:45:43.000000000 -0700
@@ -808,6 +808,12 @@ running once the system is up.
 	mpu401=		[HW,OSS]
 			Format: <io>,<irq>
 
+	msdos_skip_null_part
+			Make the MSDOS partition parsing code skip partitions
+			which have a signature of zero.  This can help with
+			mounting some incorrectly manufactured USB memory
+			devices.
+
 	MTD_Partition=	[MTD]
 			Format: <name>,<region-number>,<size>,<offset>
 
_

Re: 2.6.12-rc3 fails to read partition table

[Andries Brouwer]

On Fri, May 06, 2005 at 04:47:20AM -0700, Andrew Morton wrote:

> How about the old fallback?
> 
> So add a new boot option `msdos_skip_null_part' to enable this workaround.

Oh, horrors! Andrew, you do not want this.
This is much worse than leaving that patch or removing it.

Options are evil.
In this particular case options are terrible.

Andries

Re: 2.6.12-rc3 fails to read partition table

[Hubert Tonneau]

Hubert Tonneau wrote:
>
> 2.6.11 and 2.6.11.7 work fine.
> 2.6.12-rc1 2.6.12-rc2 and 2.6.12-rc3 fail to read partiton table on my laptop,
> also 2.6.12-rc3 works fine on another box also running FullPliant.

I tracked down the trouble to the following patch.
Partitions with type 0 are now ignored, and my hda1 single partition has been
unwisely set so.
The question might be: is it a good idea to introduce that extra constrain
in the middle of a stable serie ?

diff -urN linux-2.6.11/fs/partitions/msdos.c linux-2.6.12-rc3/fs/partitions/msdos.c
--- linux-2.6.11/fs/partitions/msdos.c	2005-03-01 23:38:12.000000000 -0800
+++ linux-2.6.12-rc3/fs/partitions/msdos.c	2005-04-20 17:03:15.000000000 -0700
@@ -114,6 +114,9 @@
 		 */
 		for (i=0; i<4; i++, p++) {
 			u32 offs, size, next;
+
+			if (SYS_IND(p) == 0)
+				continue;
 			if (!NR_SECTS(p) || is_extended_partition(p))
 				continue;
 
@@ -430,6 +433,8 @@
 	for (slot = 1 ; slot <= 4 ; slot++, p++) {
 		u32 start = START_SECT(p)*sector_size;
 		u32 size = NR_SECTS(p)*sector_size;
+		if (SYS_IND(p) == 0)
+			continue;
 		if (!size)
 			continue;
 		if (is_extended_partition(p)) {

Re: 2.6.12-rc3 fails to read partition table

[Bodo Eggert <harvested.in.lkml@posting.7eggert.dyndns.org>]

Hubert Tonneau <hubert.tonneau@fullpliant.org> wrote:
> Hubert Tonneau wrote:

>> 2.6.11 and 2.6.11.7 work fine.
>> 2.6.12-rc1 2.6.12-rc2 and 2.6.12-rc3 fail to read partiton table on my
>> laptop, also 2.6.12-rc3 works fine on another box also running FullPliant.
> 
> I tracked down the trouble to the following patch.
> Partitions with type 0 are now ignored, and my hda1 single partition has been
> unwisely set so.
> The question might be: is it a good idea to introduce that extra constrain
> in the middle of a stable serie ?

It is needed to work around other problems. Partition type 0 is usurally
considered to be empty, and some systems depend on that behaviour.
-- 
"If you see a bomb technician running, follow him."
-U.S.A.F. Ammo Tech Sgt