[PATCH] palo: fix IPL overlap with ext2/ext3 resize_inode

[PATCH] palo: fix IPL overlap with ext2/ext3 resize_inode

[James Bottomley]

palo is producing corrupt filesystems because ext2 can't cope with any
of the resize_inode, which is traditionally placed at blocks 3-258,
being in the badblocks list.  If this happens, mke2fs silently
produces a corrupt filesystem image and the palo partition will
eventually trigger a filesystem error.  The fix is to force palo to
specify -O^resize_inode to mke2fs which prevents ext2/3 from
allocating a resize_inode (and thus prevents the filesystem from being
resized).

Signed-off-by: James Bottomley <James.Bottomley@HansenPartnership.com>
---
 palo/palo.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/palo/palo.c b/palo/palo.c
index 68b85cf..e088993 100644
--- a/palo/palo.c
+++ b/palo/palo.c
@@ -443,7 +443,11 @@ do_cdrom(int media, int kernel32, int kernel64,
 #define EXT2_HOLE	((MAXBLSIZE + 1) / EXT2_BLOCKSIZE)
 
 /* offset in bytes before start of hole,  ext2 doesn't allow holes at
- * to cover the first four blocks of the filesystem */
+ * to cover the first four blocks of the filesystem
+ *
+ * Note: modern ext2/3 has a resize_inode covering blocks 3-258 so you
+ * must either always include the -O^resize_inode when creating the
+ * filesystem or define EXT2_OFFSET to (259*EXT2_BLOCKSIZE)*/
 #define EXT2_OFFSET	(4*EXT2_BLOCKSIZE)
 
 int
@@ -502,7 +506,7 @@ do_formatted(int init, int media, const char *medianame, int partition,
 	    }
 	}
 
-	sprintf(cmd, "mke2fs %s -b %d -l %s %s", do_format == 3 ? "-j" : "",
+	sprintf(cmd, "mke2fs %s -O^resize_inode -b %d -l %s %s", do_format == 3 ? "-j" : "",
 		EXT2_BLOCKSIZE, badblockfilename, partitionname);
 
 	if (verbose)
-- 
2.16.4

Re: [PATCH] palo: fix IPL overlap with ext2/ext3 resize_inode

[Helge Deller]

On 03.07.19 07:22, James Bottomley wrote:
> palo is producing corrupt filesystems because ext2 can't cope with any
> of the resize_inode, which is traditionally placed at blocks 3-258,
> being in the badblocks list.  If this happens, mke2fs silently
> produces a corrupt filesystem image and the palo partition will
> eventually trigger a filesystem error.  The fix is to force palo to
> specify -O^resize_inode to mke2fs which prevents ext2/3 from
> allocating a resize_inode (and thus prevents the filesystem from being
> resized).
>
> Signed-off-by: James Bottomley <James.Bottomley@HansenPartnership.com>
> ---
>   palo/palo.c | 8 ++++++--
>   1 file changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/palo/palo.c b/palo/palo.c
> index 68b85cf..e088993 100644
> --- a/palo/palo.c
> +++ b/palo/palo.c
> @@ -443,7 +443,11 @@ do_cdrom(int media, int kernel32, int kernel64,
>   #define EXT2_HOLE	((MAXBLSIZE + 1) / EXT2_BLOCKSIZE)
>
>   /* offset in bytes before start of hole,  ext2 doesn't allow holes at
> - * to cover the first four blocks of the filesystem */
> + * to cover the first four blocks of the filesystem
> + *
> + * Note: modern ext2/3 has a resize_inode covering blocks 3-258 so you
> + * must either always include the -O^resize_inode when creating the
> + * filesystem or define EXT2_OFFSET to (259*EXT2_BLOCKSIZE)*/
>   #define EXT2_OFFSET	(4*EXT2_BLOCKSIZE)
>
>   int
> @@ -502,7 +506,7 @@ do_formatted(int init, int media, const char *medianame, int partition,
>   	    }
>   	}
>
> -	sprintf(cmd, "mke2fs %s -b %d -l %s %s", do_format == 3 ? "-j" : "",
> +	sprintf(cmd, "mke2fs %s -O^resize_inode -b %d -l %s %s", do_format == 3 ? "-j" : "",

What happens if we face an "old" mke2fs binary which doesn't know about this feature and thus fails?
I wonder if it makes sense to add a fallback path?

Helge

Re: [PATCH] palo: fix IPL overlap with ext2/ext3 resize_inode

[James Bottomley]

On Wed, 2019-07-03 at 07:53 +0200, Helge Deller wrote:
> On 03.07.19 07:22, James Bottomley wrote:
> > palo is producing corrupt filesystems because ext2 can't cope with
> > any of the resize_inode, which is traditionally placed at blocks 3-
> > 258, being in the badblocks list.  If this happens, mke2fs silently
> > produces a corrupt filesystem image and the palo partition will
> > eventually trigger a filesystem error.  The fix is to force palo to
> > specify -O^resize_inode to mke2fs which prevents ext2/3 from
> > allocating a resize_inode (and thus prevents the filesystem from
> > being
> > resized).
> > 
> > Signed-off-by: James Bottomley <James.Bottomley@HansenPartnership.c
> > om>
> > ---
> >   palo/palo.c | 8 ++++++--
> >   1 file changed, 6 insertions(+), 2 deletions(-)
> > 
> > diff --git a/palo/palo.c b/palo/palo.c
> > index 68b85cf..e088993 100644
> > --- a/palo/palo.c
> > +++ b/palo/palo.c
> > @@ -443,7 +443,11 @@ do_cdrom(int media, int kernel32, int
> > kernel64,
> >   #define EXT2_HOLE	((MAXBLSIZE + 1) / EXT2_BLOCKSIZE)
> > 
> >   /* offset in bytes before start of hole,  ext2 doesn't allow
> > holes at
> > - * to cover the first four blocks of the filesystem */
> > + * to cover the first four blocks of the filesystem
> > + *
> > + * Note: modern ext2/3 has a resize_inode covering blocks 3-258 so
> > you
> > + * must either always include the -O^resize_inode when creating
> > the
> > + * filesystem or define EXT2_OFFSET to (259*EXT2_BLOCKSIZE)*/
> >   #define EXT2_OFFSET	(4*EXT2_BLOCKSIZE)
> > 
> >   int
> > @@ -502,7 +506,7 @@ do_formatted(int init, int media, const char
> > *medianame, int partition,
> >   	    }
> >   	}
> > 
> > -	sprintf(cmd, "mke2fs %s -b %d -l %s %s", do_format == 3 ?
> > "-j" : "",
> > +	sprintf(cmd, "mke2fs %s -O^resize_inode -b %d -l %s %s",
> > do_format == 3 ? "-j" : "",
> 
> What happens if we face an "old" mke2fs binary which doesn't know
> about this feature and thus fails?
> I wonder if it makes sense to add a fallback path?

It would fail according to my tests.  It's a bit unlikely that anyone
who's updating palo is not also updating the rest of their system, and
even when parisc fell off debian we had a version of mke2fs recent
enough to understand resize inodes because the feature was introduced
in 2006.

The other alternative is simply to change EXT2_OFFSET to always not
clash with the resize inode ... that would work for all cases.

James

Re: [PATCH] palo: fix IPL overlap with ext2/ext3 resize_inode

[Helge Deller]

On 03.07.19 16:30, James Bottomley wrote:
> On Wed, 2019-07-03 at 07:53 +0200, Helge Deller wrote:
>> On 03.07.19 07:22, James Bottomley wrote:
>>> palo is producing corrupt filesystems because ext2 can't cope with
>>> any of the resize_inode, which is traditionally placed at blocks 3-
>>> 258, being in the badblocks list.  If this happens, mke2fs silently
>>> produces a corrupt filesystem image and the palo partition will
>>> eventually trigger a filesystem error.  The fix is to force palo to
>>> specify -O^resize_inode to mke2fs which prevents ext2/3 from
>>> allocating a resize_inode (and thus prevents the filesystem from
>>> being
>>> resized).
>>>
>>> Signed-off-by: James Bottomley <James.Bottomley@HansenPartnership.c
>>> om>
>>> ---
>>>    palo/palo.c | 8 ++++++--
>>>    1 file changed, 6 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/palo/palo.c b/palo/palo.c
>>> index 68b85cf..e088993 100644
>>> --- a/palo/palo.c
>>> +++ b/palo/palo.c
>>> @@ -443,7 +443,11 @@ do_cdrom(int media, int kernel32, int
>>> kernel64,
>>>    #define EXT2_HOLE	((MAXBLSIZE + 1) / EXT2_BLOCKSIZE)
>>>
>>>    /* offset in bytes before start of hole,  ext2 doesn't allow
>>> holes at
>>> - * to cover the first four blocks of the filesystem */
>>> + * to cover the first four blocks of the filesystem
>>> + *
>>> + * Note: modern ext2/3 has a resize_inode covering blocks 3-258 so
>>> you
>>> + * must either always include the -O^resize_inode when creating
>>> the
>>> + * filesystem or define EXT2_OFFSET to (259*EXT2_BLOCKSIZE)*/
>>>    #define EXT2_OFFSET	(4*EXT2_BLOCKSIZE)
>>>
>>>    int
>>> @@ -502,7 +506,7 @@ do_formatted(int init, int media, const char
>>> *medianame, int partition,
>>>    	    }
>>>    	}
>>>
>>> -	sprintf(cmd, "mke2fs %s -b %d -l %s %s", do_format == 3 ?
>>> "-j" : "",
>>> +	sprintf(cmd, "mke2fs %s -O^resize_inode -b %d -l %s %s",
>>> do_format == 3 ? "-j" : "",
>>
>> What happens if we face an "old" mke2fs binary which doesn't know
>> about this feature and thus fails?
>> I wonder if it makes sense to add a fallback path?
>
> It would fail according to my tests.  It's a bit unlikely that anyone
> who's updating palo is not also updating the rest of their system, and
> even when parisc fell off debian we had a version of mke2fs recent
> enough to understand resize inodes because the feature was introduced
> in 2006.
>
> The other alternative is simply to change EXT2_OFFSET to always not
> clash with the resize inode ... that would work for all cases.

If mke2fs would be too old to know about the resize_inode, I'd expect
it to be able to still handle the old calling syntax (as it did before).
So my inital though would be, that if the call with -O^resize_inode fails,
we simply call the commmand again without that option.

But given that's 16 years ago when it was introduced, I think we should
be fine with your patch as it is. I'll apply it shortly.

Thanks!
Helge

Re: [PATCH] palo: fix IPL overlap with ext2/ext3 resize_inode

[Helge Deller]

On 03.07.19 16:45, Helge Deller wrote:
> On 03.07.19 16:30, James Bottomley wrote:
>> On Wed, 2019-07-03 at 07:53 +0200, Helge Deller wrote:
>>> On 03.07.19 07:22, James Bottomley wrote:
>>>> palo is producing corrupt filesystems because ext2 can't cope with
>>>> any of the resize_inode, which is traditionally placed at blocks 3-
>>>> 258, being in the badblocks list.  If this happens, mke2fs silently
>>>> produces a corrupt filesystem image and the palo partition will
>>>> eventually trigger a filesystem error.  The fix is to force palo to
>>>> specify -O^resize_inode to mke2fs which prevents ext2/3 from
>>>> allocating a resize_inode (and thus prevents the filesystem from
>>>> being
>>>> resized).
>>>>
>>>> Signed-off-by: James Bottomley <James.Bottomley@HansenPartnership.c
>>>> om>
>>>> ---
>>>>    palo/palo.c | 8 ++++++--
>>>>    1 file changed, 6 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/palo/palo.c b/palo/palo.c
>>>> index 68b85cf..e088993 100644
>>>> --- a/palo/palo.c
>>>> +++ b/palo/palo.c
>>>> @@ -443,7 +443,11 @@ do_cdrom(int media, int kernel32, int
>>>> kernel64,
>>>>    #define EXT2_HOLE    ((MAXBLSIZE + 1) / EXT2_BLOCKSIZE)
>>>>
>>>>    /* offset in bytes before start of hole,  ext2 doesn't allow
>>>> holes at
>>>> - * to cover the first four blocks of the filesystem */
>>>> + * to cover the first four blocks of the filesystem
>>>> + *
>>>> + * Note: modern ext2/3 has a resize_inode covering blocks 3-258 so
>>>> you
>>>> + * must either always include the -O^resize_inode when creating
>>>> the
>>>> + * filesystem or define EXT2_OFFSET to (259*EXT2_BLOCKSIZE)*/
>>>>    #define EXT2_OFFSET    (4*EXT2_BLOCKSIZE)
>>>>
>>>>    int
>>>> @@ -502,7 +506,7 @@ do_formatted(int init, int media, const char
>>>> *medianame, int partition,
>>>>            }
>>>>        }
>>>>
>>>> -    sprintf(cmd, "mke2fs %s -b %d -l %s %s", do_format == 3 ?
>>>> "-j" : "",
>>>> +    sprintf(cmd, "mke2fs %s -O^resize_inode -b %d -l %s %s",
>>>> do_format == 3 ? "-j" : "",

Now included in the palo v2.01 release...

Helge