Re: Question about dev_validate_header used in af_packet.c

[Xie He]

On Tue, Sep 8, 2020 at 4:53 AM Willem de Bruijn
<willemdebruijn.kernel@xxxxxxxxx> wrote:
>
> On Tue, Sep 8, 2020 at 1:04 PM Xie He <xie.he.0141@xxxxxxxxx> wrote:
> >
> > I was recently looking at some drivers, and I felt that if af_packet.c
> > could help me filter out the invalid RAW frames, I didn't need to
> > check the validity of the frames myself (in the driver when
> > transmitting). But now I guess I still need to check that.
> >
> > I feel this makes the dev_validate_header's variable-length header
> > check not very useful, because drivers need to do this check again
> > (when transmitting) anyway.
> >
> > I was thinking, after I saw dev_validate_header, that we could
> > eventually make it completely take over the responsibility for a
> > driver to validate the header when transmitting RAW frames. But now it
> > seems we would not be able to do this.
>
> Agreed. As is, it is mainly useful to block users who are ns_capable,
> but not capable.
>
> A third option is to move it behind a sysctl (with static_branch). Your
> point is valid that there really is no need for testing of drivers against
> bad packets if the data is validated directly on kernel entry.

I was thinking about this again and it came to me that maybe sometimes
people actually wanted to send invalid frames on wire (for testing the
network device on the other end of the wire)? Having thought about
this possibility I think it might be good to keep the ability for
people to have 2 choices (either having their RAW frames validated, or
not validated) through "capability" or through "sysctl" as you
mentioned. We can keep the default to be not validating the RAW frames
because RAW sockets are already intended for very special use and are
not for normal use.