inflate: cleanup input logic
Transform ugly macros to inlines
Kill mask_bits table
Eliminate magic underrun handling (dealt with by getbyte())
Signed-off-by: Matt Mackall <[email protected]>
Index: 2.6.16-rc4-inflate/lib/inflate.c
===================================================================
--- 2.6.16-rc4-inflate.orig/lib/inflate.c 2006-02-22 17:16:03.000000000 -0600
+++ 2.6.16-rc4-inflate/lib/inflate.c 2006-02-22 17:16:05.000000000 -0600
@@ -183,24 +183,23 @@ static const u16 cpdext[] = {
12, 12, 13, 13
};
-/* Macros for inflate() bit peeking and grabbing.
+/* Inlines for inflate() bit peeking and grabbing.
The usage is:
- NEEDBITS(j)
- x = b & mask_bits[j];
- DUMPBITS(j)
-
- where NEEDBITS makes sure that b has at least j bits in it, and
- DUMPBITS removes the bits from b. The macros use the variable k for
- the number of bits in b. Normally, b and k are initialized at the
- beginning of a routine that uses these macros from a global bit
- buffer and count.
+ x = readbits(&b, &k, j);
+ dumpbits(&b, &k, j);
+
+ x = pullbits(&b, &k, j);
+
+ where readbits makes sure that b has at least j bits in it, and
+ dumpbits removes the bits from b, while k tracks the number of bits
+ in b.
If we assume that EOB will be the longest code, then we will never
- ask for bits with NEEDBITS that are beyond the end of the stream.
- So, NEEDBITS should not read any more bytes than are needed to
- meet the request. Then no bytes need to be "returned" to the buffer
- at the end of the last block.
+ ask for bits that are beyond the end of the stream. So, readbits
+ should not read any more bytes than are needed to meet the request.
+ Then no bytes need to be "returned" to the buffer at the end of the
+ last block.
However, this assumption is not true for fixed blocks--the EOB code
is 7 bits, but the other literal/length codes can be 8 or 9 bits.
@@ -216,15 +215,25 @@ static const u16 cpdext[] = {
static u32 bb; /* bit buffer */
static unsigned bk; /* bits in bit buffer */
-static const u16 mask_bits[] = {
- 0x0000,
- 0x0001, 0x0003, 0x0007, 0x000f, 0x001f, 0x003f, 0x007f, 0x00ff,
- 0x01ff, 0x03ff, 0x07ff, 0x0fff, 0x1fff, 0x3fff, 0x7fff, 0xffff
-};
+static inline u32 readbits(u32 *b, u32 *k, int n)
+{
+ for( ; *k < n; *k += 8)
+ *b |= (u32)get_byte() << *k;
+ return *b & ((1 << n) - 1);
+}
-#define NEXTBYTE() ({ int v = get_byte(); if (v < 0) goto underrun; (u8)v; })
-#define NEEDBITS(n) do {while(k<(n)){b|=((u32)NEXTBYTE())<<k;k+=8;}} while(0)
-#define DUMPBITS(n) do {b>>=(n);k-=(n);} while(0)
+static inline void dumpbits(u32 *b, u32 *k, int n)
+{
+ *b >>= n;
+ *k -= n;
+}
+
+static inline u32 pullbits(u32 *b, u32 *k, int n)
+{
+ u32 r = readbits(b, k, n);
+ dumpbits(b, k, n);
+ return r;
+}
/*
Huffman code decoding is performed using a multi-level table lookup.
@@ -541,7 +550,6 @@ static int inflate_codes(struct huft *tl
unsigned n, d; /* length and index for copy */
unsigned w; /* current window position */
struct huft *t; /* pointer to table entry */
- unsigned ml, md; /* masks for bl and bd bits */
u32 b; /* bit buffer */
unsigned k; /* number of bits in bit buffer */
@@ -551,20 +559,17 @@ static int inflate_codes(struct huft *tl
w = outcnt; /* initialize window position */
/* inflate the coded data */
- ml = mask_bits[bl]; /* precompute masks for speed */
- md = mask_bits[bd];
for (;;) { /* do until end of block */
- NEEDBITS((unsigned)bl);
- if ((e = (t = tl + ((unsigned)b & ml))->e) > 16)
- do {
- if (e == 99)
- return 1;
- DUMPBITS(t->b);
- e -= 16;
- NEEDBITS(e);
- } while ((e = (t = t->v.t + ((unsigned)b &
- mask_bits[e]))->e) > 16);
- DUMPBITS(t->b);
+ t = tl + readbits(&b, &k, bl);
+ e = t->e;
+ while (e > 16) {
+ if (e == 99)
+ return 1;
+ dumpbits(&b, &k, t->b);
+ t = t->v.t + readbits(&b, &k, e - 16);
+ e = t->e;
+ }
+ dumpbits(&b, &k, t->b);
if (e == 16) { /* then it's a literal */
window[w++] = (u8)t->v.n;
if (w == WSIZE) {
@@ -577,25 +582,21 @@ static int inflate_codes(struct huft *tl
break;
/* get length of block to copy */
- NEEDBITS(e);
- n = t->v.n + ((unsigned)b & mask_bits[e]);
- DUMPBITS(e);
+ n = t->v.n + pullbits(&b, &k, e);
/* decode distance of block to copy */
- NEEDBITS((unsigned)bd);
- if ((e = (t = td + ((unsigned)b & md))->e) > 16)
- do {
- if (e == 99)
- return 1;
- DUMPBITS(t->b);
- e -= 16;
- NEEDBITS(e);
- } while ((e = (t = t->v.t + ((unsigned)b
- & mask_bits[e]))->e) > 16);
- DUMPBITS(t->b);
- NEEDBITS(e);
- d = w - t->v.n - ((unsigned)b & mask_bits[e]);
- DUMPBITS(e);
+ t = td + readbits(&b, &k, bd);
+ e = t->e;
+ while (e > 16) {
+ if (e == 99)
+ return 1;
+ dumpbits(&b, &k, t->b);
+ t = t->v.t + readbits(&b, &k, e - 16);
+ e = t->e;
+ }
+ dumpbits(&b, &k, t->b);
+
+ d = w - t->v.n - pullbits(&b, &k, e);
/* do the copy */
do {
@@ -628,9 +629,6 @@ static int inflate_codes(struct huft *tl
/* done */
return 0;
-
- underrun:
- return 4; /* Input underrun */
}
/* inflate_stored - "decompress" an inflated type 0 (stored) block. */
@@ -649,27 +647,20 @@ static int INIT inflate_stored(void)
w = outcnt; /* initialize window position */
/* go to byte boundary */
- n = k & 7;
- DUMPBITS(n);
+ dumpbits(&b, &k, k & 7);
/* get the length and its complement */
- NEEDBITS(16);
- n = ((unsigned)b & 0xffff);
- DUMPBITS(16);
- NEEDBITS(16);
- if (n != (unsigned)((~b) & 0xffff))
+ n = pullbits(&b, &k, 16);
+ if (n != (~pullbits(&b, &k, 16) & 0xffff))
return 1; /* error in compressed data */
- DUMPBITS(16);
/* read and output the compressed data */
while (n--) {
- NEEDBITS(8);
- window[w++] = (u8)b;
+ window[w++] = (u8)get_byte();
if (w == WSIZE) {
flush_output(w);
w = 0;
}
- DUMPBITS(8);
}
/* restore the globals from the locals */
@@ -679,9 +670,6 @@ static int INIT inflate_stored(void)
DEBG(">");
return 0;
-
- underrun:
- return 4; /* Input underrun */
}
@@ -748,7 +736,6 @@ static int noinline INIT inflate_dynamic
int i; /* temporary variables */
unsigned j;
unsigned l; /* last length */
- unsigned m; /* mask for bit lengths table */
unsigned n; /* number of lengths to get */
struct huft *tl; /* literal/length code table */
struct huft *td; /* distance code table */
@@ -768,26 +755,17 @@ static int noinline INIT inflate_dynamic
k = bk;
/* read in table lengths */
- NEEDBITS(5);
- nl = 257 + ((unsigned)b & 0x1f); /* number of literal/length codes */
- DUMPBITS(5);
- NEEDBITS(5);
- nd = 1 + ((unsigned)b & 0x1f); /* number of distance codes */
- DUMPBITS(5);
- NEEDBITS(4);
- nb = 4 + ((unsigned)b & 0xf); /* number of bit length codes */
- DUMPBITS(4);
+ nl = 257 + pullbits(&b, &k, 5); /* number of literal/length codes */
+ nd = 1 + pullbits(&b, &k, 5); /* number of distance codes */
+ nb = 4 + pullbits(&b, &k, 4); /* number of bit length codes */
if (nl > 286 || nd > 30)
return 1; /* bad lengths */
DEBG("dyn1 ");
/* read in bit-length-code lengths */
- for (j = 0; j < nb; j++) {
- NEEDBITS(3);
- ll[border[j]] = (unsigned)b & 7;
- DUMPBITS(3);
- }
+ for (j = 0; j < nb; j++)
+ ll[border[j]] = pullbits(&b, &k, 3);
for (; j < 19; j++)
ll[border[j]] = 0;
@@ -805,36 +783,28 @@ static int noinline INIT inflate_dynamic
/* read in literal and distance code lengths */
n = nl + nd;
- m = mask_bits[bl];
i = l = 0;
while ((unsigned)i < n) {
- NEEDBITS((unsigned)bl);
- j = (td = tl + ((unsigned)b & m))->b;
- DUMPBITS(j);
+ td = tl + readbits(&b, &k, bl);
+ dumpbits(&b, &k, td->b);
j = td->v.n;
if (j < 16) /* length of code in bits (0..15) */
ll[i++] = l = j; /* save last length in l */
else if (j == 16) { /* repeat last length 3 to 6 times */
- NEEDBITS(2);
- j = 3 + ((unsigned)b & 3);
- DUMPBITS(2);
- if ((unsigned)i + j > n)
+ j = 3 + pullbits(&b, &k, 2);
+ if ((unsigned)i + j > n)
return 1;
while (j--)
ll[i++] = l;
} else if (j == 17) { /* 3 to 10 zero length codes */
- NEEDBITS(3);
- j = 3 + ((unsigned)b & 7);
- DUMPBITS(3);
+ j = 3 + pullbits(&b, &k, 3);
if ((unsigned)i + j > n)
return 1;
while (j--)
ll[i++] = 0;
l = 0;
} else { /* j == 18: 11 to 138 zero length codes */
- NEEDBITS(7);
- j = 11 + ((unsigned)b & 0x7f);
- DUMPBITS(7);
+ j = 11 + pullbits(&b, &k, 7);
if ((unsigned)i + j > n)
return 1;
while (j--)
@@ -892,9 +862,6 @@ static int noinline INIT inflate_dynamic
DEBG(">");
return 0;
-
- underrun:
- return 4; /* Input underrun */
}
/* inflate_block - decompress a deflated block
@@ -903,28 +870,11 @@ static int noinline INIT inflate_dynamic
static int INIT inflate_block(int *e)
{
unsigned t; /* block type */
- u32 b; /* bit buffer */
- unsigned k; /* number of bits in bit buffer */
DEBG("<blk");
- /* make local bit buffer */
- b = bb;
- k = bk;
-
- /* read in last block bit */
- NEEDBITS(1);
- *e = (int)b & 1;
- DUMPBITS(1);
-
- /* read in block type */
- NEEDBITS(2);
- t = (unsigned)b & 3;
- DUMPBITS(2);
-
- /* restore the global bit buffer */
- bb = b;
- bk = k;
+ *e = pullbits(&bb, &bk, 1); /* read in last block bit */
+ t = pullbits(&bb, &bk, 2); /* read in block type */
/* inflate that block type */
if (t == 2)
@@ -938,9 +888,6 @@ static int INIT inflate_block(int *e)
/* bad block type */
return 2;
-
- underrun:
- return 4; /* Input underrun */
}
/* inflate - decompress an inflated entry */
@@ -1050,9 +997,9 @@ static int INIT gunzip(void)
u32 orig_len = 0; /* original uncompressed length */
int res;
- magic[0] = NEXTBYTE();
- magic[1] = NEXTBYTE();
- method = NEXTBYTE();
+ magic[0] = get_byte();
+ magic[1] = get_byte();
+ method = get_byte();
if (magic[0] != 037 || ((magic[1] != 0213) && (magic[1] != 0236))) {
error("bad gzip magic numbers");
@@ -1078,29 +1025,29 @@ static int INIT gunzip(void)
error("Input has invalid flags");
return -1;
}
- NEXTBYTE(); /* Get timestamp */
- NEXTBYTE();
- NEXTBYTE();
- NEXTBYTE();
+ get_byte(); /* Get timestamp */
+ get_byte();
+ get_byte();
+ get_byte();
- (void)NEXTBYTE(); /* Ignore extra flags for the moment */
- (void)NEXTBYTE(); /* Ignore OS type for the moment */
+ get_byte(); /* Ignore extra flags for the moment */
+ get_byte(); /* Ignore OS type for the moment */
if (flags & EXTRA_FIELD) {
- unsigned len = (unsigned)NEXTBYTE();
- len |= ((unsigned)NEXTBYTE()) << 8;
+ unsigned len = (unsigned)get_byte();
+ len |= ((unsigned)get_byte()) << 8;
while (len--)
- (void)NEXTBYTE();
+ get_byte();
}
/* Discard original file name if it was truncated */
if (flags & ORIG_NAME)
- while (NEXTBYTE())
+ while (get_byte())
;
/* Discard file comment if any */
if (flags & COMMENT)
- while (NEXTBYTE())
+ while (get_byte())
;
/* Decompress */
@@ -1130,15 +1077,15 @@ static int INIT gunzip(void)
/* crc32 (see algorithm.doc)
* uncompressed input size modulo 2^32
*/
- orig_crc = (u32)NEXTBYTE();
- orig_crc |= (u32)NEXTBYTE() << 8;
- orig_crc |= (u32)NEXTBYTE() << 16;
- orig_crc |= (u32)NEXTBYTE() << 24;
-
- orig_len = (u32)NEXTBYTE();
- orig_len |= (u32)NEXTBYTE() << 8;
- orig_len |= (u32)NEXTBYTE() << 16;
- orig_len |= (u32)NEXTBYTE() << 24;
+ orig_crc = (u32)get_byte();
+ orig_crc |= (u32)get_byte() << 8;
+ orig_crc |= (u32)get_byte() << 16;
+ orig_crc |= (u32)get_byte() << 24;
+
+ orig_len = (u32)get_byte();
+ orig_len |= (u32)get_byte() << 8;
+ orig_len |= (u32)get_byte() << 16;
+ orig_len |= (u32)get_byte() << 24;
/* Validate decompression */
if (orig_crc != CRC_VALUE) {
@@ -1150,8 +1097,4 @@ static int INIT gunzip(void)
return -1;
}
return 0;
-
- underrun: /* NEXTBYTE() goto's here if needed */
- error("out of input data");
- return -1;
}
On Fri, Feb 24, 2006 at 02:12:16PM -0600, Matt Mackall wrote:
> -static const u16 mask_bits[] = {
> - 0x0000,
> - 0x0001, 0x0003, 0x0007, 0x000f, 0x001f, 0x003f, 0x007f, 0x00ff,
> - 0x01ff, 0x03ff, 0x07ff, 0x0fff, 0x1fff, 0x3fff, 0x7fff, 0xffff
> -};
> +static inline u32 readbits(u32 *b, u32 *k, int n)
> +{
> + for( ; *k < n; *k += 8)
> + *b |= (u32)get_byte() << *k;
> + return *b & ((1 << n) - 1);
> +}
>
> -#define NEXTBYTE() ({ int v = get_byte(); if (v < 0) goto underrun; (u8)v; })
How does this change handle the case where we run out of input data?
This condition needs to be handled explicitly because the inflate
functions can infinitely loop.
Relying on a bit pattern returned by get_byte() is how this code
pre-fix used to work, and it caused several confused bug reports.
--
Russell King
Linux kernel 2.6 ARM Linux - http://www.arm.linux.org.uk/
maintainer of: 2.6 Serial core
On Fri, Feb 24, 2006 at 10:19:09PM +0000, Russell King wrote:
> On Fri, Feb 24, 2006 at 02:12:16PM -0600, Matt Mackall wrote:
> > -static const u16 mask_bits[] = {
> > - 0x0000,
> > - 0x0001, 0x0003, 0x0007, 0x000f, 0x001f, 0x003f, 0x007f, 0x00ff,
> > - 0x01ff, 0x03ff, 0x07ff, 0x0fff, 0x1fff, 0x3fff, 0x7fff, 0xffff
> > -};
> > +static inline u32 readbits(u32 *b, u32 *k, int n)
> > +{
> > + for( ; *k < n; *k += 8)
> > + *b |= (u32)get_byte() << *k;
> > + return *b & ((1 << n) - 1);
> > +}
> >
> > -#define NEXTBYTE() ({ int v = get_byte(); if (v < 0) goto underrun; (u8)v; })
>
> How does this change handle the case where we run out of input data?
> This condition needs to be handled explicitly because the inflate
> functions can infinitely loop.
And if you look at the current users, you'll see that only two of 15
actually use it.
> Relying on a bit pattern returned by get_byte() is how this code
> pre-fix used to work, and it caused several confused bug reports.
Just about everywhere, get_byte prints an error message and halts. In
the final refactoring, get_byte goes away and is replaced by a
->fill() method that's only called when the input buffer is emptied,
rather than byte by byte. Most of the users leave this null (since
they have the entire contents in memory already), which will give us a
nice oops. I can add another patch in the next batch that makes an
attempt to call a null ->fill instead do ->error("gunzip underrun").
--
Mathematics is the supreme nostalgia of our time.
On Sat, Feb 25, 2006 at 12:51:36AM -0600, Matt Mackall wrote:
> On Fri, Feb 24, 2006 at 10:19:09PM +0000, Russell King wrote:
> > How does this change handle the case where we run out of input data?
> > This condition needs to be handled explicitly because the inflate
> > functions can infinitely loop.
>
> And if you look at the current users, you'll see that only two of 15
> actually use it.
Sorry, I don't understand the relevance of your comment.
As the code stands in mainline, if we run out of input data, we are
guaranteed to exit from inflate.
With your change in this patch set, we no longer guaranteed to exit,
but will in some circumstances loop indefinitely.
The problem this causes is that if the ramdisk decompression runs out
of data, the kernel will just silently hang.
Please do not back out this fix.
> > Relying on a bit pattern returned by get_byte() is how this code
> > pre-fix used to work, and it caused several confused bug reports.
>
> Just about everywhere, get_byte prints an error message and halts.
And the cases where it doesn't halt is the important case.
Sorry, but I hope that this code does not get merged as is. It's
backing out a fix that I was involved in getting in, and therefore
I'm completely opposed to your code as it stands.
--
Russell King
Linux kernel 2.6 ARM Linux - http://www.arm.linux.org.uk/
maintainer of: 2.6 Serial core
On Sat, Feb 25, 2006 at 08:49:55AM +0000, Russell King wrote:
> On Sat, Feb 25, 2006 at 12:51:36AM -0600, Matt Mackall wrote:
> > On Fri, Feb 24, 2006 at 10:19:09PM +0000, Russell King wrote:
> > > How does this change handle the case where we run out of input data?
> > > This condition needs to be handled explicitly because the inflate
> > > functions can infinitely loop.
> >
> > And if you look at the current users, you'll see that only two of 15
> > actually use it.
>
> Sorry, I don't understand the relevance of your comment.
>
> As the code stands in mainline, if we run out of input data, we are
> guaranteed to exit from inflate.
>
> With your change in this patch set, we no longer guaranteed to exit,
> but will in some circumstances loop indefinitely.
>
> The problem this causes is that if the ramdisk decompression runs out
> of data, the kernel will just silently hang.
>
> Please do not back out this fix.
>
> > > Relying on a bit pattern returned by get_byte() is how this code
> > > pre-fix used to work, and it caused several confused bug reports.
> >
> > Just about everywhere, get_byte prints an error message and halts.
>
> And the cases where it doesn't halt is the important case.
>
> Sorry, but I hope that this code does not get merged as is. It's
> backing out a fix that I was involved in getting in, and therefore
> I'm completely opposed to your code as it stands.
FYI, here's the bk delta which introduced this fix.
http://linux.bkbits.net:8080/linux-2.6/diffs/lib/[email protected]?nav=index.html|src/.|src/lib|hist/lib/inflate.c
Of course, not having per-file comments in BK means that we can't get
at the cset comments which explain _why_ it is necessary. Maybe akpm
keeps an archive of such things?
--
Russell King
Linux kernel 2.6 ARM Linux - http://www.arm.linux.org.uk/
maintainer of: 2.6 Serial core
Russell King <[email protected]> wrote:
>
> FYI, here's the bk delta which introduced this fix.
>
> http://linux.bkbits.net:8080/linux-2.6/diffs/lib/[email protected]?nav=index.html|src/.|src/lib|hist/lib/inflate.c
>
> Of course, not having per-file comments in BK means that we can't get
> at the cset comments which explain _why_ it is necessary. Maybe akpm
> keeps an archive of such things?
It's easier to just google for well-chosen hunks of the patch.
http://linux.derkeiler.com/Mailing-Lists/Kernel/2003-12/4615.html
On Sat, Feb 25, 2006 at 01:04:36AM -0800, Andrew Morton wrote:
> Russell King <[email protected]> wrote:
> >
> > FYI, here's the bk delta which introduced this fix.
> >
> > http://linux.bkbits.net:8080/linux-2.6/diffs/lib/[email protected]?nav=index.html|src/.|src/lib|hist/lib/inflate.c
> >
> > Of course, not having per-file comments in BK means that we can't get
> > at the cset comments which explain _why_ it is necessary. Maybe akpm
> > keeps an archive of such things?
>
> It's easier to just google for well-chosen hunks of the patch.
>
> http://linux.derkeiler.com/Mailing-Lists/Kernel/2003-12/4615.html
Ah, thanks. I eventually found my original mail which gives a full
description of the problem:
http://www.ussg.iu.edu/hypermail/linux/kernel/0312.2/1024.html
--
Russell King
Linux kernel 2.6 ARM Linux - http://www.arm.linux.org.uk/
maintainer of: 2.6 Serial core
On Sat, Feb 25, 2006 at 08:49:55AM +0000, Russell King wrote:
> On Sat, Feb 25, 2006 at 12:51:36AM -0600, Matt Mackall wrote:
> > On Fri, Feb 24, 2006 at 10:19:09PM +0000, Russell King wrote:
> > > How does this change handle the case where we run out of input data?
> > > This condition needs to be handled explicitly because the inflate
> > > functions can infinitely loop.
> >
> > And if you look at the current users, you'll see that only two of 15
> > actually use it.
>
> Sorry, I don't understand the relevance of your comment.
The other 13 did the right thing, namely halt in get_byte. Without
adding a magic goto inside of a macro.
> Please do not back out this fix.
The backing out is only temporary, as I stated in my message. That
said, it should have never gone in.
> > > Relying on a bit pattern returned by get_byte() is how this code
> > > pre-fix used to work, and it caused several confused bug reports.
> >
> > Just about everywhere, get_byte prints an error message and halts.
>
> And the cases where it doesn't halt is the important case.
Again, current state of things. Did you read the rest of my message?
The end result is that it will halt in all cases. This code _will_not_
infinitely loop. Instead, it will dereference null or print a
diagnostic. I can add another patch to make sure it prints a nice
diagnostic in all cases if you care, but I don't think it's terribly
important.
--
Mathematics is the supreme nostalgia of our time.
On Sat, Feb 25, 2006 at 08:54:12AM -0600, Matt Mackall wrote:
> On Sat, Feb 25, 2006 at 08:49:55AM +0000, Russell King wrote:
> > On Sat, Feb 25, 2006 at 12:51:36AM -0600, Matt Mackall wrote:
> > > On Fri, Feb 24, 2006 at 10:19:09PM +0000, Russell King wrote:
> > > > How does this change handle the case where we run out of input data?
> > > > This condition needs to be handled explicitly because the inflate
> > > > functions can infinitely loop.
> > >
> > > And if you look at the current users, you'll see that only two of 15
> > > actually use it.
> >
> > Sorry, I don't understand the relevance of your comment.
>
> The other 13 did the right thing, namely halt in get_byte. Without
> adding a magic goto inside of a macro.
>
> > Please do not back out this fix.
>
> The backing out is only temporary, as I stated in my message. That
> said, it should have never gone in.
Nevertheless, it's a wilful re-introduction of a real bug.
> > > > Relying on a bit pattern returned by get_byte() is how this code
> > > > pre-fix used to work, and it caused several confused bug reports.
> > >
> > > Just about everywhere, get_byte prints an error message and halts.
> >
> > And the cases where it doesn't halt is the important case.
>
> Again, current state of things. Did you read the rest of my message?
And you're failing to see the problem.
> The end result is that it will halt in all cases. This code _will_not_
> infinitely loop. Instead, it will dereference null or print a
> diagnostic. I can add another patch to make sure it prints a nice
> diagnostic in all cases if you care, but I don't think it's terribly
> important.
Not acceptable.
We DO NOT want to halt in ALL cases. There is ONE case where we
definitely do want to GRACEFULLY fail and _NOT_ halt the kernel.
It seems that you're missing this case - the case where lib/inflate.c
is used elsewhere in the kernel apart from the boot time decompressors.
The behaviour you describe is perfectly reasonable for the boot time
decompressors, but _NOT_ once the kernel has been decompressed.
Sorry, I'm disgusted that it's come to this to get my point across.
Do I really have to use capital letters in an attempt to convey the
point?
--
Russell King
Linux kernel 2.6 ARM Linux - http://www.arm.linux.org.uk/
maintainer of: 2.6 Serial core
On Sat, Feb 25, 2006 at 06:05:21PM +0000, Russell King wrote:
> It seems that you're missing this case - the case where lib/inflate.c
> is used elsewhere in the kernel apart from the boot time decompressors.
I think you must be getting confused with lib/zlib. lib/inflate.c is
only used at boot.
--
Mathematics is the supreme nostalgia of our time.
On Sat, Feb 25, 2006 at 03:04:54PM -0600, Matt Mackall wrote:
> On Sat, Feb 25, 2006 at 06:05:21PM +0000, Russell King wrote:
> > It seems that you're missing this case - the case where lib/inflate.c
> > is used elsewhere in the kernel apart from the boot time decompressors.
>
> I think you must be getting confused with lib/zlib. lib/inflate.c is
> only used at boot.
No I'm not. Look:
$ grep -r '#include.*lib/inflate.c' [a-z]*
arch/alpha/boot/misc.c:#include "../../../lib/inflate.c"
arch/arm/boot/compressed/misc.c:#include "../../../../lib/inflate.c"
arch/arm26/boot/compressed/misc.c:#include "../../../../lib/inflate.c"
arch/cris/arch-v10/boot/compressed/misc.c:#include "../../../../../lib/inflate.c"
arch/cris/arch-v32/boot/compressed/misc.c:#include "../../../../../lib/inflate.c"
arch/i386/boot/compressed/misc.c:#include "../../../../lib/inflate.c"
arch/m32r/boot/compressed/misc.c:#include "../../../../lib/inflate.c"
arch/sh/boot/compressed/misc.c:#include "../../../../lib/inflate.c"
arch/sh64/boot/compressed/misc.c:#include "../../../../lib/inflate.c"
arch/x86_64/boot/compressed/misc.c:#include "../../../../lib/inflate.c"
All these are to do with decompressing a compressed kernel. If they
fail, halting is perfectly reasonable because we probably don't have
an executable kernel. Your arguments are fine for these. But, that's
not the full story - there are two more places where this code is
used:
init/do_mounts_rd.c:#include "../lib/inflate.c"
init/initramfs.c:#include "../lib/inflate.c"
for these your arguments that halting is fine is _NOT_ correct nor is it
desirable. The first of these is the cause of the problems both myself
and others saw, as detailed in the URL I posted previously in this thread.
Did you read that post?
--
Russell King
Linux kernel 2.6 ARM Linux - http://www.arm.linux.org.uk/
maintainer of: 2.6 Serial core
On Sat, Feb 25, 2006 at 09:22:48PM +0000, Russell King wrote:
> init/do_mounts_rd.c:#include "../lib/inflate.c"
> init/initramfs.c:#include "../lib/inflate.c"
>
> for these your arguments that halting is fine is _NOT_ correct nor is it
> desirable.
If you have an argument for why we shouldn't halt on failed
init{rd,ramfs} decompression, I look forward to hearing it.
> Did you read that post?
This? http://www.ussg.iu.edu/hypermail/linux/kernel/0312.2/1024.html
"The firmware then calls the kernel decompressor, which dutifully
decompresses the image, and calls the kernel. This image ends up
getting corrupted at some point."
Is your argument that we shouldn't halt after encountering a corrupt
image?
In my mind, being unable to decompress init* is every bit as fatal as
being unable to mount root.
--
Mathematics is the supreme nostalgia of our time.
On Sat, Feb 25, 2006 at 03:47:04PM -0600, Matt Mackall wrote:
> On Sat, Feb 25, 2006 at 09:22:48PM +0000, Russell King wrote:
> > init/do_mounts_rd.c:#include "../lib/inflate.c"
> > init/initramfs.c:#include "../lib/inflate.c"
> >
> > for these your arguments that halting is fine is _NOT_ correct nor is it
> > desirable.
>
> If you have an argument for why we shouldn't halt on failed
> init{rd,ramfs} decompression, I look forward to hearing it.
>
> > Did you read that post?
>
> This? http://www.ussg.iu.edu/hypermail/linux/kernel/0312.2/1024.html
Yes, that.
> "The firmware then calls the kernel decompressor, which dutifully
> decompresses the image, and calls the kernel. This image ends up
> getting corrupted at some point."
>
> Is your argument that we shouldn't halt after encountering a corrupt
> image?
You're getting very confused.
1. kernel is loaded.
2. firmware scans loaded kernel, finds gzip magic numbers (the compressed
kernel.)
3. firmware sets initrd pointeres to point at the compressed kernel.
4. firmware calls kernel decompressor.
5. kernel decompresses and self-relocates.
6. compressed kernel image is thereby partly corrupted.
7. kernel boots.
8. kernel tries to decompress the compressed kernel image.
9. decompressor gets confused and tries to gobble more data than is
available.
10. kernel sits there being a dumb fuck.
> In my mind, being unable to decompress init* is every bit as fatal as
> being unable to mount root.
It's very simple. With fix, the kernel successfully boots on these
machines. Without fix, the kernel hangs on these machines for _no_
good reason other than the firmware did something that was stupid.
I'm sorry, I just do not see why you're being soo bloody difficult
over this.
The buggest bloody thing about this is that _lots_ of time was wasted
on working out what the fuck was going on. I have no intention of
allowing any supposed "cleanup" to return us to the days where we have
to go though such crap again - and I will keep bitching about it until
the message gets through.
As far as I'm concerned, we're better off keeping the existing code if
this is the extent to which folk have to go to in order to preserve
needed bug fixes.
--
Russell King
Linux kernel 2.6 ARM Linux - http://www.arm.linux.org.uk/
maintainer of: 2.6 Serial core
Matt Mackall wrote:
> On Sat, Feb 25, 2006 at 09:22:48PM +0000, Russell King wrote:
>
>>init/do_mounts_rd.c:#include "../lib/inflate.c"
>>init/initramfs.c:#include "../lib/inflate.c"
>>
>>for these your arguments that halting is fine is _NOT_ correct nor is it
>>desirable.
>
>
> If you have an argument for why we shouldn't halt on failed
> init{rd,ramfs} decompression, I look forward to hearing it.
It depends on the nature of the error, and which parts were decompressed
successfully. Gzip has optional "re-sync" capability, and ideally a
decompression failure might invoke some kind of bad-block tagging
for the output instead of halting the machine. Some init{rd,ramfs}
have all the network drivers, all the SCSI drivers, all the sound
drivers, etc., but the user may care only about those for the current
machine. Other init{rd,ramfs} contain only "essential" pieces.
Even then, the pieces that are deemed more important can be at the
beginning. It might be possible to work without a sound driver,
but perhaps not without a SCSI driver.
> In my mind, being unable to decompress init* is every bit as fatal as
> being unable to mount root.
It may be possible to recover, at least partially, from one error
more than from another. It would be nice if the decompressor itself
was a minor influence instead of a major one.
--
On Sat, Feb 25, 2006 at 09:58:50PM +0000, Russell King wrote:
> On Sat, Feb 25, 2006 at 03:47:04PM -0600, Matt Mackall wrote:
> > On Sat, Feb 25, 2006 at 09:22:48PM +0000, Russell King wrote:
> > > init/do_mounts_rd.c:#include "../lib/inflate.c"
> > > init/initramfs.c:#include "../lib/inflate.c"
> > >
> > > for these your arguments that halting is fine is _NOT_ correct nor is it
> > > desirable.
> >
> > If you have an argument for why we shouldn't halt on failed
> > init{rd,ramfs} decompression, I look forward to hearing it.
> >
> > > Did you read that post?
> >
> > This? http://www.ussg.iu.edu/hypermail/linux/kernel/0312.2/1024.html
>
> Yes, that.
>
> > "The firmware then calls the kernel decompressor, which dutifully
> > decompresses the image, and calls the kernel. This image ends up
> > getting corrupted at some point."
> >
> > Is your argument that we shouldn't halt after encountering a corrupt
> > image?
>
> You're getting very confused.
>
> 1. kernel is loaded.
> 2. firmware scans loaded kernel, finds gzip magic numbers (the compressed
> kernel.)
> 3. firmware sets initrd pointeres to point at the compressed kernel.
> 4. firmware calls kernel decompressor.
> 5. kernel decompresses and self-relocates.
> 6. compressed kernel image is thereby partly corrupted.
> 7. kernel boots.
> 8. kernel tries to decompress the compressed kernel image.
> 9. decompressor gets confused and tries to gobble more data than is
> available.
> 10. kernel sits there being a dumb fuck.
Why are we attempting to decompress the kernel image again? Accident?
> > In my mind, being unable to decompress init* is every bit as fatal as
> > being unable to mount root.
>
> It's very simple. With fix, the kernel successfully boots on these
> machines. Without fix, the kernel hangs on these machines for _no_
> good reason other than the firmware did something that was stupid.
And how does this work currently? We attempt to decompress the kernel
a second time, give up and move on?
Assuming we can write to the compressed image (and thereby corrupt
it), wouldn't it be better to just stomp on the gzip magic and spare
us the overhead of decompressing it twice?
> I'm sorry, I just do not see why you're being soo bloody difficult
> over this.
Because it's taken this long to get close to an explanation of what
the problem is.
--
Mathematics is the supreme nostalgia of our time.
On Sat, Feb 25, 2006 at 04:37:37PM -0600, Matt Mackall wrote:
> On Sat, Feb 25, 2006 at 09:58:50PM +0000, Russell King wrote:
> > 1. kernel is loaded.
> > 2. firmware scans loaded kernel, finds gzip magic numbers (the compressed
> > kernel.)
> > 3. firmware sets initrd pointeres to point at the compressed kernel.
> > 4. firmware calls kernel decompressor.
> > 5. kernel decompresses and self-relocates.
> > 6. compressed kernel image is thereby partly corrupted.
> > 7. kernel boots.
> > 8. kernel tries to decompress the compressed kernel image.
> > 9. decompressor gets confused and tries to gobble more data than is
> > available.
> > 10. kernel sits there being a dumb fuck.
>
> Why are we attempting to decompress the kernel image again? Accident?
The firmware is trying to be "clever" - looking in the object it TFTP'd
for the gzip magic numbers and assuming that any it finds are an initrd.
The firmware then points the kernel at the start of that gzipped image.
The fact that a zImage contains the gzip magic numbers never occurred to
the people who implemented this misfeature in the firmware. It is a
misfeature because:
1. this exact problem - that a zImage contents can be mistaken for an initrd.
2. an initrd doesn't have to be compressed with gzip.
3. an initrd may contain gzip markers which do not relate to the start of
the initrd.
> > > In my mind, being unable to decompress init* is every bit as fatal as
> > > being unable to mount root.
> >
> > It's very simple. With fix, the kernel successfully boots on these
> > machines. Without fix, the kernel hangs on these machines for _no_
> > good reason other than the firmware did something that was stupid.
>
> And how does this work currently? We attempt to decompress the kernel
> a second time, give up and move on?
Yes.
> Assuming we can write to the compressed image (and thereby corrupt
> it), wouldn't it be better to just stomp on the gzip magic and spare
> us the overhead of decompressing it twice?
That's not guaranteed, so is impossible to do in the boot time
decompressor.
> > I'm sorry, I just do not see why you're being soo bloody difficult
> > over this.
>
> Because it's taken this long to get close to an explanation of what
> the problem is.
$#%@%#$%#@!!! I really think you're intentionally trying to wind me up
through this whole thread.
The email:
http://www.ussg.iu.edu/hypermail/linux/kernel/0312.2/1024.html
contains a full and clear explaination of the situation. The second
paragraph of that email is key to understanding the problem and makes
it absolutely clear what is trying to be decompressed as the initrd
(the corrupted compressed piggy).
--
Russell King
Linux kernel 2.6 ARM Linux - http://www.arm.linux.org.uk/
maintainer of: 2.6 Serial core
On Sat, Feb 25, 2006 at 10:57:49PM +0000, Russell King wrote:
> On Sat, Feb 25, 2006 at 04:37:37PM -0600, Matt Mackall wrote:
> > On Sat, Feb 25, 2006 at 09:58:50PM +0000, Russell King wrote:
> > > I'm sorry, I just do not see why you're being soo bloody difficult
> > > over this.
> >
> > Because it's taken this long to get close to an explanation of what
> > the problem is.
>
> $#%@%#$%#@!!! I really think you're intentionally trying to wind me up
> through this whole thread.
>
> The email:
>
> http://www.ussg.iu.edu/hypermail/linux/kernel/0312.2/1024.html
>
> contains a full and clear explaination of the situation. The second
> paragraph of that email is key to understanding the problem and makes
> it absolutely clear what is trying to be decompressed as the initrd
> (the corrupted compressed piggy).
FWIW, I didn't it either. "Work around broken boot firmware which passes
invalid initrd to kernel" would have been a simpler description.
I agree that it would be nice if inflate.c would fail gracefully
instead of halting, but why can't you just use CONFIG_BLK_DEV_INITRD=n?
Johannes
On Mon, Feb 27, 2006 at 02:18:44AM +0100, Johannes Stezenbach wrote:
> On Sat, Feb 25, 2006 at 10:57:49PM +0000, Russell King wrote:
> > The email:
> >
> > http://www.ussg.iu.edu/hypermail/linux/kernel/0312.2/1024.html
> >
> > contains a full and clear explaination of the situation. The second
> > paragraph of that email is key to understanding the problem and makes
> > it absolutely clear what is trying to be decompressed as the initrd
> > (the corrupted compressed piggy).
>
> FWIW, I didn't it either. "Work around broken boot firmware which passes
> invalid initrd to kernel" would have been a simpler description.
Sigh, I'm sick of this crap. I'm not going to debate it any further.
> I agree that it would be nice if inflate.c would fail gracefully
> instead of halting,
IT _DOES_ FAIL GRACEFULLY TODAY. WITH MATT'S PATCHES, IT _DOESN'T_.
THAT'S A REGRESSION. WHAT IS IT ABOUT THAT WHICH PEOPLE DON'T
UNDERSTAND? DO I HAVE TO SPELL IT OUT IN ONE SYLLABLE WORDS?
> but why can't you just use CONFIG_BLK_DEV_INITRD=n?
Because you might want to use an initrd for real (for installation
purposes) and therefore distributions (eg Debian) want it turned on?
Okay, this does it - I'm ignoring further discussion on this stupid
idiotic topic which is soo bloody difficult for others to understand.
--
Russell King
Linux kernel 2.6 ARM Linux - http://www.arm.linux.org.uk/
maintainer of: 2.6 Serial core
On Sat, Feb 25, 2006 at 10:57:49PM +0000, Russell King wrote:
> On Sat, Feb 25, 2006 at 04:37:37PM -0600, Matt Mackall wrote:
> > On Sat, Feb 25, 2006 at 09:58:50PM +0000, Russell King wrote:
> > > 1. kernel is loaded.
> > > 2. firmware scans loaded kernel, finds gzip magic numbers (the compressed
> > > kernel.)
> > > 3. firmware sets initrd pointeres to point at the compressed kernel.
> > > 4. firmware calls kernel decompressor.
> > > 5. kernel decompresses and self-relocates.
> > > 6. compressed kernel image is thereby partly corrupted.
> > > 7. kernel boots.
> > > 8. kernel tries to decompress the compressed kernel image.
> > > 9. decompressor gets confused and tries to gobble more data than is
> > > available.
> > > 10. kernel sits there being a dumb fuck.
> >
> > Why are we attempting to decompress the kernel image again? Accident?
>
> The firmware is trying to be "clever" - looking in the object it TFTP'd
> for the gzip magic numbers and assuming that any it finds are an initrd.
> The firmware then points the kernel at the start of that gzipped image.
>
> The fact that a zImage contains the gzip magic numbers never occurred to
> the people who implemented this misfeature in the firmware. It is a
> misfeature because:
>
> 1. this exact problem - that a zImage contents can be mistaken for an initrd.
> 2. an initrd doesn't have to be compressed with gzip.
> 3. an initrd may contain gzip markers which do not relate to the start of
> the initrd.
>
> > > > In my mind, being unable to decompress init* is every bit as fatal as
> > > > being unable to mount root.
> > >
> > > It's very simple. With fix, the kernel successfully boots on these
> > > machines. Without fix, the kernel hangs on these machines for _no_
> > > good reason other than the firmware did something that was stupid.
> >
> > And how does this work currently? We attempt to decompress the kernel
> > a second time, give up and move on?
>
> Yes.
>
> > Assuming we can write to the compressed image (and thereby corrupt
> > it), wouldn't it be better to just stomp on the gzip magic and spare
> > us the overhead of decompressing it twice?
>
> That's not guaranteed, so is impossible to do in the boot time
> decompressor.
>
> > > I'm sorry, I just do not see why you're being soo bloody difficult
> > > over this.
> >
> > Because it's taken this long to get close to an explanation of what
> > the problem is.
>
> $#%@%#$%#@!!! I really think you're intentionally trying to wind me up
> through this whole thread.
>
> The email:
>
> http://www.ussg.iu.edu/hypermail/linux/kernel/0312.2/1024.html
>
> contains a full and clear explaination of the situation. The second
> paragraph of that email is key to understanding the problem and makes
> it absolutely clear what is trying to be decompressed as the initrd
> (the corrupted compressed piggy).
It was neither full nor clear to me at least. But the above clarifies
it enough that I understand what all the fuss is about, thanks. I'm
back to the drawing board; I'll add an underflow path back in.
--
Mathematics is the supreme nostalgia of our time.
On Mon, Feb 27, 2006, Russell King wrote:
> On Mon, Feb 27, 2006 at 02:18:44AM +0100, Johannes Stezenbach wrote:
> > On Sat, Feb 25, 2006 at 10:57:49PM +0000, Russell King wrote:
> > > The email:
> > >
> > > http://www.ussg.iu.edu/hypermail/linux/kernel/0312.2/1024.html
> > >
> > > contains a full and clear explaination of the situation. The second
> > > paragraph of that email is key to understanding the problem and makes
> > > it absolutely clear what is trying to be decompressed as the initrd
> > > (the corrupted compressed piggy).
> >
> > FWIW, I didn't it either. "Work around broken boot firmware which passes
> > invalid initrd to kernel" would have been a simpler description.
>
> Sigh, I'm sick of this crap. I'm not going to debate it any further.
>
> > I agree that it would be nice if inflate.c would fail gracefully
> > instead of halting,
>
> IT _DOES_ FAIL GRACEFULLY TODAY. WITH MATT'S PATCHES, IT _DOESN'T_.
> THAT'S A REGRESSION. WHAT IS IT ABOUT THAT WHICH PEOPLE DON'T
> UNDERSTAND? DO I HAVE TO SPELL IT OUT IN ONE SYLLABLE WORDS?
I got that already, no need to shout. I just wanted to point
out that from the information you provided so far it
looks like your problem could be fixed in a more straight
forward fashion.
Problem: Boot firmware passes invalid arguments.
Solution: Ignore invalid boot firmware arguments.
> > but why can't you just use CONFIG_BLK_DEV_INITRD=n?
>
> Because you might want to use an initrd for real (for installation
> purposes) and therefore distributions (eg Debian) want it turned on?
If you use a distribution kernel which contains one, you
could simply add "noinitrd" to the kernel command line
to ignore it, no?
> Okay, this does it - I'm ignoring further discussion on this stupid
> idiotic topic which is soo bloody difficult for others to understand.
I don't understand your aggressiveness, there must be a dark
secret behind all this. Or maybe it's just the season
for flame wars.
I'm sorry to have bothered you,
Johannes
On Mon, Feb 27, 2006 at 01:07:29PM +0100, Johannes Stezenbach wrote:
> On Mon, Feb 27, 2006, Russell King wrote:
> > On Mon, Feb 27, 2006 at 02:18:44AM +0100, Johannes Stezenbach wrote:
> > > On Sat, Feb 25, 2006 at 10:57:49PM +0000, Russell King wrote:
> > > > The email:
> > > >
> > > > http://www.ussg.iu.edu/hypermail/linux/kernel/0312.2/1024.html
> > > >
> > > > contains a full and clear explaination of the situation. The second
> > > > paragraph of that email is key to understanding the problem and makes
> > > > it absolutely clear what is trying to be decompressed as the initrd
> > > > (the corrupted compressed piggy).
> > >
> > > FWIW, I didn't it either. "Work around broken boot firmware which passes
> > > invalid initrd to kernel" would have been a simpler description.
> >
> > Sigh, I'm sick of this crap. I'm not going to debate it any further.
> >
> > > I agree that it would be nice if inflate.c would fail gracefully
> > > instead of halting,
> >
> > IT _DOES_ FAIL GRACEFULLY TODAY. WITH MATT'S PATCHES, IT _DOESN'T_.
> > THAT'S A REGRESSION. WHAT IS IT ABOUT THAT WHICH PEOPLE DON'T
> > UNDERSTAND? DO I HAVE TO SPELL IT OUT IN ONE SYLLABLE WORDS?
>
> I got that already, no need to shout. I just wanted to point
> out that from the information you provided so far it
> looks like your problem could be fixed in a more straight
> forward fashion.
>
> Problem: Boot firmware passes invalid arguments.
> Solution: Ignore invalid boot firmware arguments.
Let me try to explain - but I doubt it'll do any good because folk don't
seem to understand plain English here anymore (or at least that's what
it seems like from _my_ perspective.)
In order to detect that the arguments are invalid, you'd need to validate
the initrd.
In order to validate a compressed initrd, you'd have to trial-inflate it,
just like gunzip -t.
(a) gunzip -t is able to work because it has setjmp/longjmp, so when it
runs out of data, it can sanely exit from the data reading function
when it encounters insufficient data.
The kernel does not have such functionality, and it has been determined
long ago that the kernel shall not have such functionality - it was
discussed at the time when this problem first came up and the resounding
answer was precisely as I state.
(b) if we have to have separate code to validate a compressed image, that is
a complete waste of code and resources - we already have something which
tests whether a compressed image is valid by inflating it - called
lib/inflate.c.
So, your suggestion isn't a really a solution when the simple solution
is to keep the original _simple_ fix for a buggy integration of the gzip
inflate code.
> > > but why can't you just use CONFIG_BLK_DEV_INITRD=n?
> >
> > Because you might want to use an initrd for real (for installation
> > purposes) and therefore distributions (eg Debian) want it turned on?
>
> If you use a distribution kernel which contains one, you
> could simply add "noinitrd" to the kernel command line
> to ignore it, no?
Tell that to all the people who have complained in the past about it.
> > Okay, this does it - I'm ignoring further discussion on this stupid
> > idiotic topic which is soo bloody difficult for others to understand.
>
> I don't understand your aggressiveness, there must be a dark
> secret behind all this. Or maybe it's just the season
> for flame wars.
I'm completely and utterly pissed off with this thread, having to almost
go back to kindergarten type explainations to get the point across.
_That's_ what has been soo infuriating about this whole saga.
And what's even more stupid is the attitude that required fixes can be
thrown out of the kernel, and then a massive argument is required to
re-explain wtf they're necessary.
Are we doomed to have to repeatedly explain why bug fixes are necessary?
If that's the case, let's pack up this Linux kernel thing because we're
on a route to insanity.
--
Russell King
Linux kernel 2.6 ARM Linux - http://www.arm.linux.org.uk/
maintainer of: 2.6 Serial core
Followup to: <[email protected]>
By author: Russell King <[email protected]>
In newsgroup: linux.dev.kernel
>
> All these are to do with decompressing a compressed kernel. If they
> fail, halting is perfectly reasonable because we probably don't have
> an executable kernel. Your arguments are fine for these. But, that's
> not the full story - there are two more places where this code is
> used:
>
> init/do_mounts_rd.c:#include "../lib/inflate.c"
> init/initramfs.c:#include "../lib/inflate.c"
>
> for these your arguments that halting is fine is _NOT_ correct nor is it
> desirable. The first of these is the cause of the problems both myself
> and others saw, as detailed in the URL I posted previously in this thread.
> Did you read that post?
>
These probably should use lib/zlib instead...
-hpa
On Tue, Mar 07, 2006 at 03:26:05PM -0800, H. Peter Anvin wrote:
> Followup to: <[email protected]>
> By author: Russell King <[email protected]>
> In newsgroup: linux.dev.kernel
> >
> > All these are to do with decompressing a compressed kernel. If they
> > fail, halting is perfectly reasonable because we probably don't have
> > an executable kernel. Your arguments are fine for these. But, that's
> > not the full story - there are two more places where this code is
> > used:
> >
> > init/do_mounts_rd.c:#include "../lib/inflate.c"
> > init/initramfs.c:#include "../lib/inflate.c"
> >
> > for these your arguments that halting is fine is _NOT_ correct nor is it
> > desirable. The first of these is the cause of the problems both myself
> > and others saw, as detailed in the URL I posted previously in this thread.
> > Did you read that post?
> >
>
> These probably should use lib/zlib instead...
That's a reasonable suggestion. I have an eventual goal of getting
down to only one (cleaned up) zlib instance in the kernel and I'm
somewhat more appalled by the lib/zlib/inflate code so I haven't gone
that route.
--
Mathematics is the supreme nostalgia of our time.