netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Hangbin Liu <liuhangbin@gmail.com>
To: David Ahern <dsahern@gmail.com>
Cc: Sabrina Dubroca <sd@queasysnail.net>,
	netdev@vger.kernel.org, Roopa Prabhu <roopa@cumulusnetworks.com>,
	"David S . Miller" <davem@davemloft.net>
Subject: Re: [PATCH net] ipv4: Add ICMPv6 support when parse route ipproto
Date: Tue, 26 Feb 2019 10:17:39 +0800	[thread overview]
Message-ID: <20190226021739.GS10051@dhcp-12-139.nay.redhat.com> (raw)
In-Reply-To: <212750de-2d6a-ebb9-5079-bacf234f6cf6@gmail.com>

On Mon, Feb 25, 2019 at 11:46:51AM -0700, David Ahern wrote:
> On 2/25/19 4:14 AM, Sabrina Dubroca wrote:
> > 2019-02-25, 15:47:00 +0800, Hangbin Liu wrote:
> >> @@ -14,6 +15,7 @@ int rtm_getroute_parse_ip_proto(struct nlattr *attr, u8 *ip_proto,
> >>  	case IPPROTO_TCP:
> >>  	case IPPROTO_UDP:
> >>  	case IPPROTO_ICMP:
> >> +	case IPPROTO_ICMPV6:
> > 
> > Is IPPROTO_ICMPV6 supposed to be valid in the IPv4 code path calling
> > this function? 
> 
> no. I do not see how that makes sense.

I also thought about this issue. Currently we didn't check the ipproto in both
IPv4 and IPv6. You can set icmp in ip6 rules or icmpv6 in ipv4 rules.
This looks don't make any serious problem. It's just a user mis-configuration,
the kernel check the proto number and won't match normal IP/IPv6 headers.

But yes, we should make it more strict, do you think if I should add a new
rtm_getroute_parse_ip6_proto() function, or just add a family parameter
in previous function?

The #if IS_ENABLED(CONFIG_IPV6) guardian makes sense. I will fix it.

Thanks
Hangbin

  reply	other threads:[~2019-02-26  2:17 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-02-25  7:47 [PATCH net] ipv4: Add ICMPv6 support when parse route ipproto Hangbin Liu
2019-02-25 11:14 ` Sabrina Dubroca
2019-02-25 18:46   ` David Ahern
2019-02-26  2:17     ` Hangbin Liu [this message]
2019-02-26  2:23       ` David Ahern
2019-02-26  3:48         ` Hangbin Liu
2019-02-26  9:06           ` Sabrina Dubroca
2019-02-27  8:15 ` [PATCH v2 " Hangbin Liu
2019-03-02  0:42   ` David Miller

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20190226021739.GS10051@dhcp-12-139.nay.redhat.com \
    --to=liuhangbin@gmail.com \
    --cc=davem@davemloft.net \
    --cc=dsahern@gmail.com \
    --cc=netdev@vger.kernel.org \
    --cc=roopa@cumulusnetworks.com \
    --cc=sd@queasysnail.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).