> If the application has provided fewer than hard_header_len bytes,
> dev_validate_header() will zero out the skb->data as needed. This is
> acceptable for SOCK_DGRAM/PF_PACKET sockets but in all other cases,

This was added not for datagram sockets, but to be able to bypass
validation. See the message in commit 2793a23aacbd ("net: validate
variable length ll header") and discussion leading up to that patch.

> the application must provide a full L2 header, and the PF_PACKET Tx
> paths must fail with an error when fewer than hard_header_len bytes
> are detected.

As David pointed out, this does not handle variable length headers
correctly. In link layers that support these, hard_header_len defines
the maximum header length. A hard failure on len < hard_header_len
would be incorrect.

The ->validate callback was added to allow specifying additional
constraints on a per protocol basis. This is where a min constraint
can be added, e.g., for ethernet.

> All invocations to dev_validate_header() already adjusts the
> skb's data, len, tail etc pointers based on hard_header_len before
> invoking dev_validate_header(), so additional skb pointers should
> not be needed after dev_validate_header().
>
> Signed-off-by: Sowmini Varadhan <sowmini.varad...@oracle.com>
> ---

> -       if (!dev_validate_header(dev, skb->data, len)) {
> +       newlen = dev_validate_header(dev, skb->data, len);
> +       /* As comments above this function indicate, a full L2 header
> +        * must be passed to this function, so if newlen > len, bail.
> +        */
> +       if (newlen < 0 || newlen > len) {

If callers only care whether the function returned failure or
increased len, which also indicates failure, it is cleaner to leave it
a boolean and fail in cases where len < the minimum for that link
layer type. No caller actually uses newlen.

> +               /* Caller has allocated for copylen in non-paged part of
> +                * skb so we should never find newlen > hdrlen
> +                */
> +               WARN_ON(newlen > hdrlen);

WARN_ON_ONCE is safer.

Reply via email to