[net-next] net: Only honor ifindex in IP_PKTINFO if non-0

Message ID 20180216190303.30769-1-dsahern@gmail.com
State Accepted
Delegated to: David Miller
Headers show
Series
  • [net-next] net: Only honor ifindex in IP_PKTINFO if non-0
Related show

Commit Message

David Ahern Feb. 16, 2018, 7:03 p.m.
Only allow ifindex from IP_PKTINFO to override SO_BINDTODEVICE settings
if the index is actually set in the message.

Signed-off-by: David Ahern <dsahern@gmail.com>
---
 net/ipv4/ip_sockglue.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

Comments

David Miller Feb. 16, 2018, 9:43 p.m. | #1
From: David Ahern <dsahern@gmail.com>
Date: Fri, 16 Feb 2018 11:03:03 -0800

> Only allow ifindex from IP_PKTINFO to override SO_BINDTODEVICE settings
> if the index is actually set in the message.
> 
> Signed-off-by: David Ahern <dsahern@gmail.com>

Ok, this behavior meets reasonable expectations, applied, thanks.

None of the documation is clear about this relationship between
ip_pktinfo's ifindex and settings made by SO_BINDTODEVICE.
David Ahern Feb. 16, 2018, 10:12 p.m. | #2
On 2/16/18 2:43 PM, David Miller wrote:
> From: David Ahern <dsahern@gmail.com>
> Date: Fri, 16 Feb 2018 11:03:03 -0800
> 
>> Only allow ifindex from IP_PKTINFO to override SO_BINDTODEVICE settings
>> if the index is actually set in the message.
>>
>> Signed-off-by: David Ahern <dsahern@gmail.com>
> 
> Ok, this behavior meets reasonable expectations, applied, thanks.
> 
> None of the documation is clear about this relationship between
> ip_pktinfo's ifindex and settings made by SO_BINDTODEVICE.
> 

It is my understanding that SO_BINDTODEVICE is the strongest -- it
requires admin to set. From there IP_PKTINFO and IP_UNICAST_IF are
non-root options and hence weaker. If that is the proper expectation,
then the right thing to do is probably to error out if ipc.oif is
already set. I was concerned that would break existing apps, so this
seemed to be a compromise.
David Ahern March 6, 2018, 8:52 p.m. | #3
On 2/16/18 12:03 PM, David Ahern wrote:
> Only allow ifindex from IP_PKTINFO to override SO_BINDTODEVICE settings
> if the index is actually set in the message.
> 
> Signed-off-by: David Ahern <dsahern@gmail.com>
> ---
>  net/ipv4/ip_sockglue.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 

Dave: Any chance this can get backported to 4.14 and 4.9 stable
releases? Right now, net-snmp is dead in the water with VRF because it
uses IP_PKTINFO to set the source address AND in the process sets the
ifindex to 0 to allow asymmetric routing (request received on one
interface and response goes out a different one). This patch allows 'ip
vrf exec mgmt snmpd' to work.
David Miller March 6, 2018, 9:41 p.m. | #4
From: David Ahern <dsahern@gmail.com>
Date: Tue, 6 Mar 2018 13:52:05 -0700

> On 2/16/18 12:03 PM, David Ahern wrote:
>> Only allow ifindex from IP_PKTINFO to override SO_BINDTODEVICE settings
>> if the index is actually set in the message.
>> 
>> Signed-off-by: David Ahern <dsahern@gmail.com>
>> ---
>>  net/ipv4/ip_sockglue.c | 6 ++++--
>>  1 file changed, 4 insertions(+), 2 deletions(-)
>> 
> 
> Dave: Any chance this can get backported to 4.14 and 4.9 stable
> releases? Right now, net-snmp is dead in the water with VRF because it
> uses IP_PKTINFO to set the source address AND in the process sets the
> ifindex to 0 to allow asymmetric routing (request received on one
> interface and response goes out a different one). This patch allows 'ip
> vrf exec mgmt snmpd' to work.

Ok, I've put this patch into 'net' and will let it soak there, and
also queue it up for a future -stable submission.

Thanks David.

Patch

diff --git a/net/ipv4/ip_sockglue.c b/net/ipv4/ip_sockglue.c
index 008be04ac1cc..9dca0fb8c482 100644
--- a/net/ipv4/ip_sockglue.c
+++ b/net/ipv4/ip_sockglue.c
@@ -258,7 +258,8 @@  int ip_cmsg_send(struct sock *sk, struct msghdr *msg, struct ipcm_cookie *ipc,
 			src_info = (struct in6_pktinfo *)CMSG_DATA(cmsg);
 			if (!ipv6_addr_v4mapped(&src_info->ipi6_addr))
 				return -EINVAL;
-			ipc->oif = src_info->ipi6_ifindex;
+			if (src_info->ipi6_ifindex)
+				ipc->oif = src_info->ipi6_ifindex;
 			ipc->addr = src_info->ipi6_addr.s6_addr32[3];
 			continue;
 		}
@@ -288,7 +289,8 @@  int ip_cmsg_send(struct sock *sk, struct msghdr *msg, struct ipcm_cookie *ipc,
 			if (cmsg->cmsg_len != CMSG_LEN(sizeof(struct in_pktinfo)))
 				return -EINVAL;
 			info = (struct in_pktinfo *)CMSG_DATA(cmsg);
-			ipc->oif = info->ipi_ifindex;
+			if (info->ipi_ifindex)
+				ipc->oif = info->ipi_ifindex;
 			ipc->addr = info->ipi_spec_dst.s_addr;
 			break;
 		}