On Sun, Oct 11, 2020 at 12:11 PM Cong Wang <xiyou.wangc...@gmail.com> wrote: > > GRE tunnel has its own header_ops, ipgre_header_ops, and sets it > conditionally. When it is set, it assumes the outer IP header is > already created before ipgre_xmit(). > > This is not true when we send packets through a raw packet socket, > where L2 headers are supposed to be constructed by user. Packet > socket calls dev_validate_header() to validate the header. But > GRE tunnel does not set dev->hard_header_len, so that check can > be simply bypassed, therefore uninit memory could be passed down > to ipgre_xmit(). Similar for dev->needed_headroom. > > dev->hard_header_len is supposed to be the length of the header > created by dev->header_ops->create(), so it should be used whenever > header_ops is set, and dev->needed_headroom should be used when it > is not set.
Hi, thanks for attempting to fix this tunnel. Are we still considering removing header_ops->create? As said in my email sent previously today, I want to remove header_ops->create because 1) this keeps the un-exposed headers of GRE devices consistent with those of GRETAP devices, and 2) I think the GRE header (and the headers before the GRE header) is not actually the L2 header of the tunnel (the Wikipedia page for "Generic Routing Encapsulation" doesn't consider this protocol to be at L2 either). I'm not sure if you still agree to remove header_ops->create. Do you still agree but think it'd be better to do that in a separate patch? Removing header_ops->create would simplify the fixing of the issue you are trying to fix, too, because that way we would no longer need to use header_ops or hard_header_len. Also, I'm worried that changing hard_header_len (or needed_headroom) in ipgre_link_update would have racing issues. If we remove header_ops, we no longer need to use hard_header_len and we can just set needed_headroom to the maximum value, so that we no longer need to update them in ipgre_link_update.
