After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 709230 - rdisc: mask out non-prefix bit from network prefix
rdisc: mask out non-prefix bit from network prefix
Status: RESOLVED FIXED
Product: NetworkManager
Classification: Platform
Component: IP and DNS config
git master
Other Linux
: Normal normal
: ---
Assigned To: NetworkManager maintainer(s)
NetworkManager maintainer(s)
Depends on:
Blocks:
 
 
Reported: 2013-10-01 20:32 UTC by Pavel Simerda
Modified: 2013-10-03 18:54 UTC
See Also:
GNOME target: ---
GNOME version: ---


Description Pavel Simerda 2013-10-01 20:32:19 UTC 
Some router advertisement daemons allow setting an incorrect prefix address that has non-zero bits in the host part. This can be easily worked around client-side by setting all host part bits to zero.

See downstream bug:

https://bugzilla.redhat.com/show_bug.cgi?id=1008104
Comment 1 Dan Williams 2013-10-02 23:04:03 UTC 
How about dcbw/rh1008104-bgo709230-rdisc-prefix-mask ?
Comment 2 Thomas Haller 2013-10-03 00:03:46 UTC 
It looks correct to me. Maybe you could g_return_if_false that 0<=plen<=128 ?
Comment 3 Pavel Simerda 2013-10-03 15:20:12 UTC 
(In reply to comment #1)
> How about dcbw/rh1008104-bgo709230-rdisc-prefix-mask ?

Looks good, can be pushed. Sorry for delaying it.

You could optionally simplify the code:

route.plen = ndp_msg_opt_prefix_len (msg, offset);
route.network = mask_address (*ndp_msg_opt_prefix (msg, offset), route.plen);

(In reply to comment #2)
> It looks correct to me. Maybe you could g_return_if_false that 0<=plen<=128 ?

+1 except that guint doesn't need the lower bound.
Comment 4 Dan Williams 2013-10-03 18:26:29 UTC 
(In reply to comment #3)
> (In reply to comment #1)
> > How about dcbw/rh1008104-bgo709230-rdisc-prefix-mask ?
> 
> Looks good, can be pushed. Sorry for delaying it.
> 
> You could optionally simplify the code:
> 
> route.plen = ndp_msg_opt_prefix_len (msg, offset);
> route.network = mask_address (*ndp_msg_opt_prefix (msg, offset), route.plen);

Done.

> (In reply to comment #2)
> > It looks correct to me. Maybe you could g_return_if_false that 0<=plen<=128 ?
> 
> +1 except that guint doesn't need the lower bound.

Done, without a lower bound.  Also switched it to guint8 because that's the return type from libndp.

Also, the routes need the same treatment, so I masked them too.  But that caused the compiler to yell about mask_address() returning an "aggregate value", I assume becuase with one callsite it's able to inline automatically, but with two callsites it doesn't?  Anyway, fixed that up too.

Works on x86 with my radvd test setup and mis-configuring both the prefix and an advertised route; fails without my fixes.
Comment 5 Dan Williams 2013-10-03 18:54:37 UTC 
Ack from danw, merged to master.