| Message ID | 6c039e090908040036j38666152y2db1d4c55529eaff@mail.gmail.com |
|---|---|
| State | Rejected, archived |
| Delegated to: | David Miller |
| Headers | show |
Chia-chi Yeh (葉家齊) пишет: > After thinking more deeply, I believe that IPv6 does the right thing > and IPv4 does not. SO_BINDTODEVICE requires CAP_NET_RAW, so it is a > privileged operation. Therefore, it looks weird to me if one can > specify other interface than the bound one without the same > capability. The following patch makes the behavior in IPv4 and IPv6 > identical. Thanks for your help. > > Chia-chi > > --- a/net/ipv4/ip_sockglue.c 2009-08-04 15:11:39.000000000 +0800 > +++ b/net/ipv4/ip_sockglue.c 2009-08-04 15:17:05.000000000 +0800 > @@ -213,7 +213,11 @@ > 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) { > + if (ipc->oif && info->ipi_ifindex != ipc->oif) > + return -EINVAL; > + ipc->oif = info->ipi_ifindex; Hello Sorry if its my mistake or i someone not understand :) if (ipc->oif && info->ipi_ifindex != ipc->oif) // if match: info->ipi_ifindex != ipc->oif return ... else // else match: info->ipi_ifindex == ipc->oif // but you do ipc->oif = info->ipi_ifindex; // why if you else match allready check for it? Thanks > + } > ipc->addr = info->ipi_spec_dst.s_addr; > break; > } > > On Tue, Aug 4, 2009 at 12:23 PM, David Miller<davem@davemloft.net> wrote: >> From: John Dykstra <john.dykstra1@gmail.com> >> Date: Wed, 29 Jul 2009 19:10:21 -0500 >> >>> I guess Dave's letting this stand. I'm posting this just to make sure >>> this is an explicit decision. >> I'm still thinking about this. >> > -- > To unsubscribe from this list: send the line "unsubscribe netdev" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > > -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, Aug 4, 2009 at 3:57 PM, Badalian Vyacheslav<slavon@bigtelecom.ru> wrote: > > Hello > Sorry if its my mistake or i someone not understand :) > > if (ipc->oif && info->ipi_ifindex != ipc->oif) > // if match: info->ipi_ifindex != ipc->oif > return ... > else > // else match: info->ipi_ifindex == ipc->oif > // but you do > ipc->oif = info->ipi_ifindex; > // why if you else match allready check for it? > > Thanks Hi Badalian, Consider ipc->oif = 0 before the if-condition. :) Chia-chi -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
From: Chia-chi Yeh (葉家齊) <chiachi@android.com> Date: Tue, 4 Aug 2009 15:36:46 +0800 > After thinking more deeply, I believe that IPv6 does the right thing > and IPv4 does not. SO_BINDTODEVICE requires CAP_NET_RAW, so it is a > privileged operation. Therefore, it looks weird to me if one can > specify other interface than the bound one without the same > capability. The following patch makes the behavior in IPv4 and IPv6 > identical. Thanks for your help. I think we really cannot change behavior here. If the user specifies "0" in ipi_ifindex we must respect that in ipc->oif. This is an override, and the ability to override is the very purpose of this control message. Even GLIBC makes use of that case of specifying "0" in ipi_ifindex. We must respect it. I'm not applying any of these patches, sorry. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
2009/8/5 David Miller <davem@davemloft.net>: > I think we really cannot change behavior here. If the user specifies > "0" in ipi_ifindex we must respect that in ipc->oif. This is an > override, and the ability to override is the very purpose of this > control message. > > Even GLIBC makes use of that case of specifying "0" in ipi_ifindex. > We must respect it. > > I'm not applying any of these patches, sorry. > If you treat ipi_ifindex as an override, do you want to do that in ipi6_ifindex as well? Also, CAP_NET_RAW check for SO_BINDTODEVICE becomes meaningless in this case. I did not find the usage of ipi_ifindex in glibc. It would be great if you can give me some pointers. Thanks for your help. Chia-chi -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
From: Chia-chi Yeh (葉家齊) <chiachi@android.com> Date: Wed, 5 Aug 2009 08:06:25 +0800 > 2009/8/5 David Miller <davem@davemloft.net>: >> I think we really cannot change behavior here. If the user specifies >> "0" in ipi_ifindex we must respect that in ipc->oif. This is an >> override, and the ability to override is the very purpose of this >> control message. >> >> Even GLIBC makes use of that case of specifying "0" in ipi_ifindex. >> We must respect it. >> >> I'm not applying any of these patches, sorry. >> > > If you treat ipi_ifindex as an override, do you want to do that in > ipi6_ifindex as well? Also, CAP_NET_RAW check for SO_BINDTODEVICE > becomes meaningless in this case. ipi_ifindex influences routing decisions, it doesn't also influence socket matching like SO_BINDTODEVICE does. That's the crucial difference. This interface index gets passed into the routing lookup and that is completely harmless to allow users to do. About ipv6 it is indeed an important issue, because apps that work now with ipv4's behavior are going to break if they care about this case when they start using ipv6. Probably the thing to do is make ipv6 work the same as ipv4. > I did not find the usage of ipi_ifindex in glibc. It would be great if > you can give me some pointers. Thanks for your help. If grep is broken on your system, I'm sorry to hear that: find . -type f | xargs egrep IP_PKTINFO Give that a try on your glibc tree.
--- a/net/ipv4/ip_sockglue.c 2009-08-04 15:11:39.000000000 +0800 +++ b/net/ipv4/ip_sockglue.c 2009-08-04 15:17:05.000000000 +0800 @@ -213,7 +213,11 @@ 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) { + if (ipc->oif && info->ipi_ifindex != ipc->oif) + return -EINVAL; + ipc->oif = info->ipi_ifindex; + } ipc->addr = info->ipi_spec_dst.s_addr; break;
After thinking more deeply, I believe that IPv6 does the right thing and IPv4 does not. SO_BINDTODEVICE requires CAP_NET_RAW, so it is a privileged operation. Therefore, it looks weird to me if one can specify other interface than the bound one without the same capability. The following patch makes the behavior in IPv4 and IPv6 identical. Thanks for your help. Chia-chi } On Tue, Aug 4, 2009 at 12:23 PM, David Miller<davem@davemloft.net> wrote: > From: John Dykstra <john.dykstra1@gmail.com> > Date: Wed, 29 Jul 2009 19:10:21 -0500 > >> I guess Dave's letting this stand. I'm posting this just to make sure >> this is an explicit decision. > > I'm still thinking about this. > -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html