* [PATCH net] ipv4: fix broadcast packets reception
@ 2016-03-21 15:42 Paolo Abeni
2016-03-21 15:50 ` David Miller
0 siblings, 1 reply; 3+ messages in thread
From: Paolo Abeni @ 2016-03-21 15:42 UTC (permalink / raw)
To: netdev
Cc: David S. Miller, Alexey Kuznetsov, Shawn Bohrer,
Hannes Frederic Sowa
Currently, ingress ipv4 broadcast datagrams are dropped if the
ingress interface lacks an ipv4 address. This is caused by
multiple issues:
- in udp_v4_early_demux() ip_check_mc_rcu is invoked even on
bcast packets
- ip_route_input_slow() always try to validate the source
This patch tries to address both issues, invoking ip_check_mc_rcu()
only for mcast packets and calling fib_validate_source() only
if the in_device has an address, at least.
Fixes: 6e5403093261 ("ipv4/udp: Verify multicast group is ours in upd_v4_early_demux()")
Signed-off-by: Paolo Abeni <pabeni@redhat•com>
---
net/ipv4/route.c | 6 +++++-
net/ipv4/udp.c | 12 ++++++++----
2 files changed, 13 insertions(+), 5 deletions(-)
diff --git a/net/ipv4/route.c b/net/ipv4/route.c
index 02c6229..e86d33d 100644
--- a/net/ipv4/route.c
+++ b/net/ipv4/route.c
@@ -1867,7 +1867,11 @@ brd_input:
if (skb->protocol != htons(ETH_P_IP))
goto e_inval;
- if (!ipv4_is_zeronet(saddr)) {
+ /* Do not validate the source if this device not not have an
+ * IPv4 address yet and the destination is the broadcast address.
+ */
+ if (!ipv4_is_zeronet(saddr) && (in_dev->ifa_list ||
+ !ipv4_is_lbcast(daddr))) {
err = fib_validate_source(skb, saddr, 0, tos, 0, dev,
in_dev, &itag);
if (err < 0)
diff --git a/net/ipv4/udp.c b/net/ipv4/udp.c
index 836abe5..08eed5e 100644
--- a/net/ipv4/udp.c
+++ b/net/ipv4/udp.c
@@ -2070,10 +2070,14 @@ void udp_v4_early_demux(struct sk_buff *skb)
if (!in_dev)
return;
- ours = ip_check_mc_rcu(in_dev, iph->daddr, iph->saddr,
- iph->protocol);
- if (!ours)
- return;
+ /* we are supposed to accept bcast packets */
+ if (skb->pkt_type == PACKET_MULTICAST) {
+ ours = ip_check_mc_rcu(in_dev, iph->daddr, iph->saddr,
+ iph->protocol);
+ if (!ours)
+ return;
+ }
+
sk = __udp4_lib_mcast_demux_lookup(net, uh->dest, iph->daddr,
uh->source, iph->saddr, dif);
} else if (skb->pkt_type == PACKET_HOST) {
--
1.8.3.1
^ permalink raw reply related [flat|nested] 3+ messages in thread* Re: [PATCH net] ipv4: fix broadcast packets reception
2016-03-21 15:42 [PATCH net] ipv4: fix broadcast packets reception Paolo Abeni
@ 2016-03-21 15:50 ` David Miller
2016-03-21 16:12 ` Paolo Abeni
0 siblings, 1 reply; 3+ messages in thread
From: David Miller @ 2016-03-21 15:50 UTC (permalink / raw)
To: pabeni; +Cc: netdev, kuznet, sbohrer, hannes
From: Paolo Abeni <pabeni@redhat•com>
Date: Mon, 21 Mar 2016 16:42:11 +0100
> Currently, ingress ipv4 broadcast datagrams are dropped if the
> ingress interface lacks an ipv4 address. This is caused by
> multiple issues:
>
> - in udp_v4_early_demux() ip_check_mc_rcu is invoked even on
> bcast packets
>
> - ip_route_input_slow() always try to validate the source
>
> This patch tries to address both issues, invoking ip_check_mc_rcu()
> only for mcast packets and calling fib_validate_source() only
> if the in_device has an address, at least.
>
> Fixes: 6e5403093261 ("ipv4/udp: Verify multicast group is ours in upd_v4_early_demux()")
> Signed-off-by: Paolo Abeni <pabeni@redhat•com>
I'm extremely weary to change the routing lookup code wrt. broadcast, multicast,
etc. policies, ever. The checks in there have multiple decades of precedence
and therefore are extremely dangerous to modify.
The UDP change in question didn't touch the generic routing code, therfore you
must fix this bug without modifying it either.
Sorry.
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [PATCH net] ipv4: fix broadcast packets reception
2016-03-21 15:50 ` David Miller
@ 2016-03-21 16:12 ` Paolo Abeni
0 siblings, 0 replies; 3+ messages in thread
From: Paolo Abeni @ 2016-03-21 16:12 UTC (permalink / raw)
To: David Miller; +Cc: netdev, kuznet, sbohrer, hannes
On Mon, 2016-03-21 at 11:50 -0400, David Miller wrote:
> From: Paolo Abeni <pabeni@redhat•com>
> Date: Mon, 21 Mar 2016 16:42:11 +0100
>
> > Currently, ingress ipv4 broadcast datagrams are dropped if the
> > ingress interface lacks an ipv4 address. This is caused by
> > multiple issues:
> >
> > - in udp_v4_early_demux() ip_check_mc_rcu is invoked even on
> > bcast packets
> >
> > - ip_route_input_slow() always try to validate the source
> >
> > This patch tries to address both issues, invoking ip_check_mc_rcu()
> > only for mcast packets and calling fib_validate_source() only
> > if the in_device has an address, at least.
> >
> > Fixes: 6e5403093261 ("ipv4/udp: Verify multicast group is ours in upd_v4_early_demux()")
> > Signed-off-by: Paolo Abeni <pabeni@redhat•com>
>
> I'm extremely weary to change the routing lookup code wrt. broadcast, multicast,
> etc. policies, ever. The checks in there have multiple decades of precedence
> and therefore are extremely dangerous to modify.
>
> The UDP change in question didn't touch the generic routing code, therfore you
> must fix this bug without modifying it either.
ok, I'll try to find something less intrusive. I'm not sure if it will
be possible.
Just a little addendum: the current issue is not caused only by the
commit 6e5403093261, but also by some less trivial/older change into the
routing lookup code I was unable to track exactly.
Cheers,
Paolo
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2016-03-21 16:12 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-03-21 15:42 [PATCH net] ipv4: fix broadcast packets reception Paolo Abeni
2016-03-21 15:50 ` David Miller
2016-03-21 16:12 ` Paolo Abeni
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox