Re: Revert 462fb2af9788a82a534f8184abfde31574e1cfa0 (bridge : Sanitize skb before it enters the IP stack)

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Florian Westphal schreef op 19/05/2014 19:09:
David Newall <davidn@xxxxxxxxxxxxxxx> wrote:

[ remove lkml and cc nf-devel ]

I tried to persevere with the commit: I recalculated checksum, which
left routes and times improperly updated in options.  Then I tried
calling ip_forward_options, which looks like it would correctly
update RR and TS (not to mention checksum)m but that bombed because
skb_rtable returned NULL.

Yes.  bridge<->netfilter wiring is pure duct tape.
The glue code will set up a fake rtable for the skb after the
prerouting hook. [ see br_nf_pre_routing_finish() ].

I see three ways to progress:

1. Possibly call ip_forward_option, but that requires somebody who
understands this code to help;
2. Just recalculate the checksum, leaving crap in the options; or
3. Revert the commit.

I think none of these are an option.

I fail to understand why a bridge should honor/modifiy IP options.

For the 'local delivery' case the ip stack will take care of
option parsing, for forwarding it should be sufficient to do
sanity tests (for netfilters sake).

From a quick glance, it should be sufficient to edit
br_parse_ip_options() and remove everything after

memset(IPCB(skb), 0, sizeof(struct inet_skb_parm));

A 2nd step would be to move a copy of ip_options_compile()
into br_netfilter.c and trim it down to only validate the
ipv4 header without modifying it.

Perhaps it's possible to call ip_options_compile with a skb == NULL, like ip_options.c::ip_options_get_finish does. That way we don't need to duplicate code. An alternative would be to make sure that the data pointed to by IPCB and BR_INPUT_SKB_CB don't overlap. If this were the case, we could indeed just revert the commit that was referred to.


cheers,
Bart

--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [Netfitler Users]     [LARTC]     [Bugtraq]     [Yosemite Forum]

  Powered by Linux