On Fri, Aug 31, 2012 at 06:35:14PM +0200, Oliver Hartkopp wrote:

Hello Kurt and Wolfram,

On 31.08.2012 14:50, Kurt Van Dijck wrote:

quoted

On Fri, Aug 31, 2012 at 01:59:04PM +0200, Wolfram Sang wrote:

quoted

Hi,

I just noticed today that commit c7cd606f60e7679c7f9eee7010f02a6f000209c1
("can: Fix data length code handling in rx path") says regarding
overlong packets:

===

The ISO 11898-1 Chapter 8.4.2.3 (DLC field) says that register values > 8
should be reduced to 8 without any error reporting or frame drop.

===

and this is why get_can_dlc() came into existance.

However, functions like can_dropped_invalid_skb() from dev.h or can_rcv() from
af_can.c do check for a correct length, but drop the packet in the error case.
Don't those need to be fixed?

Interesting point. My first idea is to acknowledge your thinking, BUT:
* ISO 11898-1 is about the wire format, whereas your issues address
  higher level interfaces. Higher level interfaces may IMHO use a stricter
  format that the wire format.

Pointing to the wire format is a good idea.
To me this "dlc error ignoring" requirement is implemented by the CAN
controller. And when the dlc value in the controller register is not sanitized
we should do it on driver level.

If I understand both of you correctly, that means the checks in the
functions I mentioned should be removed then?

Fine with me if this is consistent, I just wondered if it wasn't nice to
have a check in an upper layer in case the driver did not correctly
implement the check.

Thanks,

   Wolfram

-- 
Pengutronix e.K.                           | Wolfram Sang                |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |