| Message ID | 20171019082108.40D7AA0EDF@unicorn.suse.cz |
|---|---|
| State | Accepted, archived |
| Delegated to: | stephen hemminger |
| Headers | show |
| Series | [iproute2] ip maddr: fix filtering by device | expand |
On Thu, Oct 19, 2017 at 10:21:08AM +0200, Michal Kubecek wrote: > Commit 530903dd9003 ("ip: fix igmp parsing when iface is long") uses > variable len to keep trailing colon from interface name comparison. This > variable is local to loop body but we set it in one pass and use it in > following one(s) so that we are actually using (pseudo)random length for > comparison. This became apparent since commit b48a1161f5f9 ("ipmaddr: Avoid > accessing uninitialized data") always initializes len to zero so that the > name comparison is always true. As a result, "ip maddr show dev eth0" shows > IPv4 multicast addresses for all interfaces. > > Instead of keeping the length, let's simply replace the trailing colon with > a null byte. The bonus is that we get correct interface name in ma.name. > > Fixes: 530903dd9003 ("ip: fix igmp parsing when iface is long") > Signed-off-by: Michal Kubecek <mkubecek@suse.cz> Acked-by: Phil Sutter <phil@nwl.cc>
> On Thu, Oct 19, 2017 at 10:21:08AM +0200, Michal Kubecek wrote: > > Commit 530903dd9003 ("ip: fix igmp parsing when iface is long") uses > > variable len to keep trailing colon from interface name comparison. This > > variable is local to loop body but we set it in one pass and use it in > > following one(s) so that we are actually using (pseudo)random length for > > comparison. This became apparent since commit b48a1161f5f9 ("ipmaddr: Avoid > > accessing uninitialized data") always initializes len to zero so that the > > name comparison is always true. As a result, "ip maddr show dev eth0" shows > > IPv4 multicast addresses for all interfaces. > > Instead of keeping the length, let's simply replace the trailing colon with > > a null byte. The bonus is that we get correct interface name in ma.name. > > Fixes: 530903dd9003 ("ip: fix igmp parsing when iface is long") > > Signed-off-by: Michal Kubecek <mkubecek@suse.cz> > Acked-by: Phil Sutter <phil@nwl.cc> Acked-by: Petr Vorel <pvorel@suse.cz> Thanks for fixing, Michal. Petr
On Thu, 19 Oct 2017 10:21:08 +0200 (CEST) Michal Kubecek <mkubecek@suse.cz> wrote: > Commit 530903dd9003 ("ip: fix igmp parsing when iface is long") uses > variable len to keep trailing colon from interface name comparison. This > variable is local to loop body but we set it in one pass and use it in > following one(s) so that we are actually using (pseudo)random length for > comparison. This became apparent since commit b48a1161f5f9 ("ipmaddr: Avoid > accessing uninitialized data") always initializes len to zero so that the > name comparison is always true. As a result, "ip maddr show dev eth0" shows > IPv4 multicast addresses for all interfaces. > > Instead of keeping the length, let's simply replace the trailing colon with > a null byte. The bonus is that we get correct interface name in ma.name. > > Fixes: 530903dd9003 ("ip: fix igmp parsing when iface is long") > Signed-off-by: Michal Kubecek <mkubecek@suse.cz> Applied
diff --git a/ip/ipmaddr.c b/ip/ipmaddr.c index 5683f6fa830c..46b86a3a7723 100644 --- a/ip/ipmaddr.c +++ b/ip/ipmaddr.c @@ -136,17 +136,18 @@ static void read_igmp(struct ma_info **result_p) while (fgets(buf, sizeof(buf), fp)) { struct ma_info *ma; - size_t len = 0; if (buf[0] != '\t') { + size_t len; + sscanf(buf, "%d%s", &m.index, m.name); len = strlen(m.name); if (m.name[len - 1] == ':') - len--; + m.name[len - 1] = '\0'; continue; } - if (filter.dev && strncmp(filter.dev, m.name, len)) + if (filter.dev && strcmp(filter.dev, m.name)) continue; sscanf(buf, "%08x%d", (__u32 *)&m.addr.data, &m.users);
Commit 530903dd9003 ("ip: fix igmp parsing when iface is long") uses variable len to keep trailing colon from interface name comparison. This variable is local to loop body but we set it in one pass and use it in following one(s) so that we are actually using (pseudo)random length for comparison. This became apparent since commit b48a1161f5f9 ("ipmaddr: Avoid accessing uninitialized data") always initializes len to zero so that the name comparison is always true. As a result, "ip maddr show dev eth0" shows IPv4 multicast addresses for all interfaces. Instead of keeping the length, let's simply replace the trailing colon with a null byte. The bonus is that we get correct interface name in ma.name. Fixes: 530903dd9003 ("ip: fix igmp parsing when iface is long") Signed-off-by: Michal Kubecek <mkubecek@suse.cz> --- ip/ipmaddr.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-)