Kris Katterjohn wrote:
From: Patrick McHardy
Thats even worse, it will fall through to the second switch statement.
Just call BUG(), since it means you forgot to check something.
I'm 99.99% sure I didn't forget anything. I just left it there in my first patch
because it was there before. Everything is checked for in sk_chk_filter(), so
the default branch will never be used. And if you think we need the default
branch, why call BUG instead of just returning 0 as before?
Because it means a check was missing. We had a number of bad bugs
in there before, better to find them quick than have them lurking
around until someone does an code audit. But in this case I tend
to agree with you, it doesn't seem to affect anything critical.
Ah, I missed that you already moved the check for div-by-constant
in a previous patch. So the entire point of this patch is to remove
the default-branch in the switch-statement? That hardly seems worth
the additional amount of code.
No. The point of this patch is to stop a bad filter instruction from causing the
kernel to loop in sk_run_filter(), but always return 0. If we check to make sure
only valid instructions are used in sk_chk_filter(), then we eliminate that
problem.
That seems to be a purely theoretical problem, the common case is
correctly compiled filters from libpcap. BTW: I haven't read the
BPF specs/paper/whatever, but if this is documented behaviour,
something might actually be relying on it.
My idea is to check for everything in sk_chk_filter(). Check everything
there and make sure that when sk_run_filter() is called, it shouldn't have any
problems. I moved the check for constant-division-by-zero to sk_chk_filter()
because it could be checked there. Now, I think a check for only valid
instructions should be there too, because we can go ahead and check it there.
sk_chk_filter() should return an error if the filter is bad, not allow it and
have sk_run_filter() give you nothing.
Well, I'm not convinced that there is a problem, but I do agree that its
good to return an error for invalid filters - if they are actually
invalid and its not documented behaviour to accept unknown instructions
and treat them as return.
-
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
