netdev - Re: [RFC][PATCH] ipv6 multicast forwarding: Remove threshold checking and some trivial bugs

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Mon, 17 Dec 2012 21:03:33 -0800 (PST)
From:	David Miller <davem@...emloft.net>
To:	wcang@....wide.ad.jp
Cc:	netdev@...r.kernel.org, yoshfuji@...ux-ipv6.org
Subject: Re: [RFC][PATCH] ipv6 multicast forwarding: Remove threshold
 checking and some trivial bugs

From: Ang Way Chuang <wcang@....wide.ad.jp>
Date: Tue, 18 Dec 2012 12:57:11 +0800

> This patch fixes trivial bugs for IPv6 multicast forwarding code and remove
> threshold checking for multicast forwarding cache.
> 
> 1. Threshold checking in IPv6 multicast forwarding cache (MFC) was not properly implemented.
> syscall to setsockopt(... MRT6_ADD_MIF,...) doesn't affect the TTL because it is never used.
> In fact, all MFC will always have ttl of 1 as set by ip6mr_mfc_add. From my limited knowledge of
> multicast routing, threshold setting on interface is only used by DVMRP which doesn't support
> IPv6. FreeBSD's struct mif6ctl doesn't have vifc_threshold. This patch removes the ttl cruft
> within kernel. Userspace ABI for backward compatibility. Can someone knowledgable in multicast
> routing please verify whether my understanding is correct?
> 2. Don't allow addition of MFC with non-existent multicast interface index.
> 3. Don't allow addition of MFC where incoming interface is part of oif list. This does not make
>    sense. Why would we want to send a multicast back to the interface where it originates from.
> 4. setsockopt(....MRT6_ADD_MIF, ) allows a "physical" interface to be registered as multicast 
>    interface multiple times. This doesn't make sense. Don't allow registration duplicate 
>    registration of the same "physical" interface. 
> 
> This patch has been tested, albeit minimally using a simple program. Is this patch okay for
> inclusion? Will sign off if it is okay.

How about we don't mix together a set of bug fixes, with a semantic
change (the removal of the threshold checking)?

I also don't see what the point is of not signing off on this change
when you submit it.

If you delay the signoff until after review, you're just causing it to
take longer to have your changes integrated.  It also makes it look
like you didn't believe fully in your change, so you probably should
have sent it as an RFC and listed your doubts in the email instead.

Overall I would rate this as an extremely poor patch submission,
sorry.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html