cvs commit: src/sys/ufs/ffs ffs_alloc.c

[Bruce Evans]

On Mon, 1 Oct 2007, Bjoern A. Zeeb wrote:

> On Mon, 1 Oct 2007, Bruce Evans wrote:
>
>> On Mon, 1 Oct 2007, Bjoern A. Zeeb wrote:
>>>> s/fis/fix/
>>> 
>>> yes it should. I closed the PR, See the comment there.
>> 
>> s/fix/work around/
>> 
>> The bug is in newfs and tunefs permitting garbage parameters, so it cannot
>> be fixed in ffs_alloc.c.
>
> No matter what iput the kernel gets and from where, it MUST NOT (or at
> least SHOULD not;) panic unless explicitly request by KASSERT/panic/..

Not quite true.  There are hundreds or thousands of sysctls that can
be used to set critical parameters to garbage which will cause a panic.
Bounds checking for sysctl parameters is almost completely absent, and
this is sometimes useful for investigating the limits of useful parameters
without rebuilding the kernel.  Also, a division by zero trap is preferable
to a panic since it is restartable.

> So this commit fixes a DIV0 bug in the kernel.
>
> Of course you are right, that the values should be checked by the tools
> that we have in the tree so that this problem would not occur.
> We could even check if the values given make sense at all, but that still
> is a different story to a kernel panic.

You deleted the part where I said where the fix belongs in the kernel
(next to related fixups).

ffs does almost no runtime checking by design.  It depends on fsck or
mount-time fixups doing all the necessary checking and fixups, so that
the main code can be simpler and faster.  New code shouldn't do things
differently.

Bruce