Re: [RFC] sctp/tcp: Question -- ICMPv4 length check (not) redundant?

[Vlad Yasevich <vladislav.yasevich@hp.com>]

Gerrit Renker wrote:
> | >  * only TCP/SCTP seem to have a proper per-protocol "payload too short" test;
> | 
> | Hm..  In the standard case, these do seem to be redundant since 8 bytes are required
> | by ICMP spec.
> | 
> | >  * for DCCP, the work is actually doubled since 
> | >    - first the ICMP handler tests for minimally 8 bytes, 
> | >    - then the DCCP error handler tests for required minimum of 12 bytes.
> | 
> | DCCP and any other protocol that requires more error data should check for it in
> | its own handler.  8 bytes should be guaranteed to such handler.
> | 
> | What am I missing?
> | 
> As per last message, please disregard the patch suggestion made at the
> beginning of the thread.
> 
> In TCP, the 8 bytes happen to be enough for doing sequence number checks. Other
> protocols have different header lengths and semantics. Thus doing the checks
> at the transport layer makes more sense than in the ICMP handler.
> 
> RFC 1122 is almost 20 years old, from a time before IPcomp, SCTP, or DCCP.

So the suggestion really is then to remove the length check icmp_unreach()?

Because as it stands right now, the protocol error handler will not be invoked
if we don't have the iphdr + 8 bytes worth of data.  That's is actually a requirement
from the ICMP rfc 792.

Seems that not including those 8 bytes is a violation of that spec.  So, icmp_unreach()
should keep current code and jump to out_err upon failure.

Each upper layer protocol that requires additional space, should validate the existence
of that additional space and handle it appropriately.

It doesn't matter that IPcomp header is only 4 bytes.  ICMP error requires ihlen + 8 bytes and 
at least that much should be provided  Otherwise, it a malformed ICMP packet.

-vlad