netdev - Re: [Patch net-next] ipv6: add missing /proc/sys/net/ipv6/ping_group

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 for Android: free password hash cracker in your pocket

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

Date:	Sun, 9 Jun 2013 18:42:20 +0900
From:	Lorenzo Colitti <lorenzo@...gle.com>
To:	Cong Wang <amwang@...hat.com>
Cc:	"netdev@...r.kernel.org" <netdev@...r.kernel.org>,
	"David S. Miller" <davem@...emloft.net>
Subject: Re: [Patch net-next] ipv6: add missing /proc/sys/net/ipv6/ping_group_range

On Sat, Jun 8, 2013 at 6:54 PM, Cong Wang <amwang@...hat.com> wrote:
> commit 6d0bfe22611602f366 (net: ipv6: Add IPv6 support to the ping socket.)
> adds IPv6 ping socket support, but forgot to create
> /proc/sys/net/ipv6/ping_group_range, therefore it wrongly shares
> /proc/sys/net/ipv4/ping_group_range with IPv4.

As you said, I'm not sure that sharing the sysctl file with IPv4 is a
problem. Is it useful to support different permissions for IPv4 ping
versus IPv6 ping? I would imagine that if an admin wants certain
groups to be able to run ping, those groups would be the same for IPv4
ping and IPv6 ping. But I'm not the authority here. Maybe David has an
opinion?

That said - if we do want to do this, I think you should find a way to
do it with less code duplication. For example, your newly-added
ping_v6_init_sock is almost line-for-line identical with the current
ping_init_sock. This adds maintenance burden, because any future fix
needs to modify both functions, and creates the opportunity for the
two functions to diverge in the future, leading to different behaviour
and/or bugs in the future. It should be trivial to refactor the common
code out into one function that is called by both the IPv4 and the
IPv6 init functions.

There is similar code duplication in your new get_ping_group range and
in other places as well.
--
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