Re: [PATCH 3/7] inflate pt1: clean up input logic

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
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

Relevant Pages

  • Re: Chrony on an Isolated Machine
    ... >> Also look in the config file for your kernel (some put it into ... You created and compiled that rtl kernel somewhere. ... >> You do not want to remove halt altogether. ... It uses the hwclock command. ...
    (comp.os.linux.networking)
  • Mandrake 9.1 and 9.2 install problem
    ... It halt the system, no response from keyboard, I have to reboot it. ... I then tried the alt1 kernel, which is the kernel 2.2.19, it works ... The Mandrake 9.1 error database showed that there is a bug in kernel ... file to install from network, but it halt at same stage as 9.1 does, ...
    (comp.os.linux.misc)
  • Re: init troubles with custom install .. perhaps a kernel bug?
    ... > mount a remote share from which to run scripts installing a ghost ... > information I get is when in verbose logging mode, it reaches the halt ... however it seems that the kernel process terminates ... I also inserted a print statement at the start of init, ...
    (freebsd-hackers)
  • Re: [PATCH 3/7] inflate pt1: clean up input logic
    ... This condition needs to be handled explicitly because the inflate ... functions can infinitely loop. ... The other 13 did the right thing, namely halt in get_byte. ...
    (Linux-Kernel)
  • Re: Introducing a poweroff(8) command
    ... Add a hard link to rebootcalled "poweroff" which defaults to the ... same behavior as "halt -p". ... .Op Fl k Ar kernel ... The file system cache is not flushed. ...
    (freebsd-arch)