[PATCH] fs/partitions/msdos.c Guard against negative sizes

[PATCH] fs/partitions/msdos.c Guard against negative sizes

[Ph. Marek]

Hello Andries,

I've recently had a partly destroyed pc where the partition size was negative. 
I was at that time running a dos recovery disk and having severe problems - 
it would not boot.

So I had the idea of looking into the linux kernel to see how it would handle 
these. I wrote some checks in fs/partitions/msdos.c.
I'm not sure if just dropping this partitions is the right thing to do - but 
as I don't know what could break if there are wrong entries I propose the 
following patch: (incomplete - should check in many more places)


If there is a better way of handling this I'd like to know.


Regards,

Phil



diff -u linux-2.5.59/fs/partitions/msdos.c.orig 
linux-2.5.59/fs/partitions/msdos.c
--- linux-2.5.59/fs/partitions/msdos.c.orig     Mon Jan 27 12:22:45 2003
+++ linux-2.5.59/fs/partitions/msdos.c  Mon Jan 27 12:24:28 2003
@@ -133,6 +133,8 @@
                           these sometimes contain random garbage */
                        offs = START_SECT(p)*sector_size;
                        size = NR_SECTS(p)*sector_size;
+                       if (size<0)
+                               continue;
                        next = this_sector + offs;
                        if (i >= 2) {
                                if (offs + size > this_size)
@@ -423,7 +425,7 @@
        for (slot = 1 ; slot <= 4 ; slot++, p++) {
                u32 start = START_SECT(p)*sector_size;
                u32 size = NR_SECTS(p)*sector_size;
-               if (!size)
+               if (size <= 0)
                        continue;
                if (is_extended_partition(p)) {
                        /* prevent someone doing mkfs or mkswap on an

Re: [PATCH] fs/partitions/msdos.c Guard against negative sizes

[Andries Brouwer]

[ach, I didnt pay attention, had not seen that this was cc-ed to l-k;
let me repeat on l-k]

On Mon, Jan 27, 2003 at 12:30:05PM +0100, Ph. Marek wrote:

>                 u32 size = NR_SECTS(p)*sector_size;
> -               if (!size)
> +               if (size <= 0)

This doesnt work since size is unsigned.

Andries

Re: [PATCH] fs/partitions/msdos.c Guard against negative sizes

[Andries.Brouwer]

    u32 start = START_SECT(p)*sector_size;
    u32 size = NR_SECTS(p)*sector_size;
    And this looks as if it was calculated in bytes

But it is not. It converts hardware sector size units
to 512-byte units. For example, some MO disks use
2048-byte sectors, and also have the partition tables
in that unit.

    What about overflow?

More detailed checking of the table is certainly possible.

[But then, weird tables only occur when people fiddle with
the partition table themselves. Or when an accident happened.
I am not sure inhibiting access to partitions that look strange
is useful. It might make rescue operations difficult or impossible.]

    And btw, when can a partition that extends beyond the end
    be "allowed or even necessary"?

One reason is that "the end" is not well-defined. E.g.,
disk manufacturers invent jumpers that make the disk appear
smaller than it is in reality in order to avoid BIOS bugs.
See http://www.win.tue.nl/~aeb/linux/Large-Disk-11.html#ss11.3

In many cases the kernel option CONFIG_IDEDISK_STROKE will tell
the kernel to automatically fix up fake lengths. If that doesn't
work, a user mode utility may be needed to give the disk full size.
Since that utility is run after the kernel does the partition detection,
it is often easiest to let the last logical partition start just before
the fake end.

Andries

Re: [PATCH] fs/partitions/msdos.c Guard against negative sizes

[Ph. Marek]

> >             u32 size = NR_SECTS(p)*sector_size;
> > -           if (!size)
> > +           if (size <= 0)
>
> But an unsigned variable cannot be negative, so what you write
> is the same as what was there already.
>
> In other words, your "negative" sizes are just very large positive.
>
> The test should be whether the partition extends beyond the end
> of the disk, but sometimes that is allowed, even necessary, so
> there could be a warning only.
Sorry, didn't pay enough attention.
On second look: it's an u32.
But that brings the next question up: what about overflow? u32 means a max of 
4GB.
	u32 start = START_SECT(p)*sector_size;
	u32 size = NR_SECTS(p)*sector_size;
And this looks as if it was calculated in bytes - but I'm sure that 4GB is not 
the max size! Where am I wrong??

And btw, when can a partition that extends beyond the end be "allowed or even 
necessary"?


Thanks for your answer!


Regards,

Phil