On Tue, Sep 8, 2020 at 4:53 AM Willem de Bruijn <willemdebruijn.ker...@gmail.com> wrote: > > On Tue, Sep 8, 2020 at 1:04 PM Xie He <xie.he.0...@gmail.com> 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.