* [PATCH net] ipv4: fix wildcard search with inet_confirm_addr() @ 2013-11-08 14:37 Nicolas Dichtel 2013-11-08 21:24 ` Julian Anastasov 0 siblings, 1 reply; 7+ messages in thread From: Nicolas Dichtel @ 2013-11-08 14:37 UTC (permalink / raw) To: davem; +Cc: netdev, Nicolas Dichtel Help of this function says: "in_dev: only on this interface, 0=any interface", but in fact the code supposes that it will never be NULL. Note that this function was never called with in_dev == NULL, but it's exported and may be used by an external module. Signed-off-by: Nicolas Dichtel <nicolas.dichtel@6wind.com> --- This bug was spotted by code review. drivers/net/bonding/bonding.h | 10 +++------- include/linux/inetdevice.h | 3 ++- net/ipv4/arp.c | 3 ++- net/ipv4/devinet.c | 8 ++++---- 4 files changed, 11 insertions(+), 13 deletions(-) diff --git a/drivers/net/bonding/bonding.h b/drivers/net/bonding/bonding.h index 03cf3fd14490..448895e90ccb 100644 --- a/drivers/net/bonding/bonding.h +++ b/drivers/net/bonding/bonding.h @@ -407,15 +407,11 @@ static inline bool bond_is_slave_inactive(struct slave *slave) static inline __be32 bond_confirm_addr(struct net_device *dev, __be32 dst, __be32 local) { - struct in_device *in_dev; - __be32 addr = 0; + __be32 addr; rcu_read_lock(); - in_dev = __in_dev_get_rcu(dev); - - if (in_dev) - addr = inet_confirm_addr(in_dev, dst, local, RT_SCOPE_HOST); - + addr = inet_confirm_addr(dev_net(dev), __in_dev_get_rcu(dev), dst, + local, RT_SCOPE_HOST); rcu_read_unlock(); return addr; } diff --git a/include/linux/inetdevice.h b/include/linux/inetdevice.h index 79640e015a86..5870f7060917 100644 --- a/include/linux/inetdevice.h +++ b/include/linux/inetdevice.h @@ -164,7 +164,8 @@ extern int devinet_ioctl(struct net *net, unsigned int cmd, void __user *); extern void devinet_init(void); extern struct in_device *inetdev_by_index(struct net *, int); extern __be32 inet_select_addr(const struct net_device *dev, __be32 dst, int scope); -extern __be32 inet_confirm_addr(struct in_device *in_dev, __be32 dst, __be32 local, int scope); +__be32 inet_confirm_addr(struct net *net, struct in_device *in_dev, __be32 dst, + __be32 local, int scope); extern struct in_ifaddr *inet_ifa_byprefix(struct in_device *in_dev, __be32 prefix, __be32 mask); static __inline__ int inet_ifa_match(__be32 addr, struct in_ifaddr *ifa) diff --git a/net/ipv4/arp.c b/net/ipv4/arp.c index 7808093cede6..fc5ebc7e1962 100644 --- a/net/ipv4/arp.c +++ b/net/ipv4/arp.c @@ -408,7 +408,8 @@ static int arp_ignore(struct in_device *in_dev, __be32 sip, __be32 tip) default: return 0; } - return !inet_confirm_addr(in_dev, sip, tip, scope); + return !inet_confirm_addr(dev_net(in_dev->dev), in_dev, sip, tip, + scope); } static int arp_filter(__be32 sip, __be32 tip, struct net_device *dev) diff --git a/net/ipv4/devinet.c b/net/ipv4/devinet.c index a1b5bcbd04ae..47cdb035f817 100644 --- a/net/ipv4/devinet.c +++ b/net/ipv4/devinet.c @@ -1236,22 +1236,22 @@ static __be32 confirm_addr_indev(struct in_device *in_dev, __be32 dst, /* * Confirm that local IP address exists using wildcards: + * - net: netns to check, cannot be NULL * - in_dev: only on this interface, 0=any interface * - dst: only in the same subnet as dst, 0=any dst * - local: address, 0=autoselect the local address * - scope: maximum allowed scope value for the local address */ -__be32 inet_confirm_addr(struct in_device *in_dev, +__be32 inet_confirm_addr(struct net *net, struct in_device *in_dev, __be32 dst, __be32 local, int scope) { __be32 addr = 0; struct net_device *dev; - struct net *net; if (scope != RT_SCOPE_LINK) - return confirm_addr_indev(in_dev, dst, local, scope); + return in_dev ? confirm_addr_indev(in_dev, dst, local, scope) : + 0; - net = dev_net(in_dev->dev); rcu_read_lock(); for_each_netdev_rcu(net, dev) { in_dev = __in_dev_get_rcu(dev); -- 1.8.4.1 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH net] ipv4: fix wildcard search with inet_confirm_addr() 2013-11-08 14:37 [PATCH net] ipv4: fix wildcard search with inet_confirm_addr() Nicolas Dichtel @ 2013-11-08 21:24 ` Julian Anastasov 2013-11-13 15:23 ` [PATCH net v2] " Nicolas Dichtel 0 siblings, 1 reply; 7+ messages in thread From: Julian Anastasov @ 2013-11-08 21:24 UTC (permalink / raw) To: Nicolas Dichtel; +Cc: davem, netdev Hello, On Fri, 8 Nov 2013, Nicolas Dichtel wrote: > Help of this function says: "in_dev: only on this interface, 0=any interface", > but in fact the code supposes that it will never be NULL. > > Note that this function was never called with in_dev == NULL, but it's exported > and may be used by an external module. The initial idea was that arp_ignore=3 can provide dev=NULL. Later argument was changed to in_dev but we should allow the NULL value. As you are going to change the arguments may be we can partially revert commit 39a6d06300128d32f361f4f790beba0ca83730eb, i.e. to add the missing in_dev = NULL in arp_ignore() and also revert this change: - if (in_dev != NULL) + if (scope != RT_SCOPE_LINK) > Signed-off-by: Nicolas Dichtel <nicolas.dichtel@6wind.com> > --- > > This bug was spotted by code review. > > drivers/net/bonding/bonding.h | 10 +++------- > include/linux/inetdevice.h | 3 ++- > net/ipv4/arp.c | 3 ++- > net/ipv4/devinet.c | 8 ++++---- > 4 files changed, 11 insertions(+), 13 deletions(-) > > diff --git a/drivers/net/bonding/bonding.h b/drivers/net/bonding/bonding.h > index 03cf3fd14490..448895e90ccb 100644 > --- a/drivers/net/bonding/bonding.h > +++ b/drivers/net/bonding/bonding.h > @@ -407,15 +407,11 @@ static inline bool bond_is_slave_inactive(struct slave *slave) > > static inline __be32 bond_confirm_addr(struct net_device *dev, __be32 dst, __be32 local) > { > - struct in_device *in_dev; > - __be32 addr = 0; > + __be32 addr; > > rcu_read_lock(); > - in_dev = __in_dev_get_rcu(dev); > - > - if (in_dev) > - addr = inet_confirm_addr(in_dev, dst, local, RT_SCOPE_HOST); > - > + addr = inet_confirm_addr(dev_net(dev), __in_dev_get_rcu(dev), dst, > + local, RT_SCOPE_HOST); As in_dev = NULL should be allowed, above change will not be needed, we should keep the 'if (in_dev)' there. > rcu_read_unlock(); > return addr; > } > diff --git a/include/linux/inetdevice.h b/include/linux/inetdevice.h > index 79640e015a86..5870f7060917 100644 > --- a/include/linux/inetdevice.h > +++ b/include/linux/inetdevice.h > @@ -164,7 +164,8 @@ extern int devinet_ioctl(struct net *net, unsigned int cmd, void __user *); > extern void devinet_init(void); > extern struct in_device *inetdev_by_index(struct net *, int); > extern __be32 inet_select_addr(const struct net_device *dev, __be32 dst, int scope); > -extern __be32 inet_confirm_addr(struct in_device *in_dev, __be32 dst, __be32 local, int scope); > +__be32 inet_confirm_addr(struct net *net, struct in_device *in_dev, __be32 dst, > + __be32 local, int scope); > extern struct in_ifaddr *inet_ifa_byprefix(struct in_device *in_dev, __be32 prefix, __be32 mask); > > static __inline__ int inet_ifa_match(__be32 addr, struct in_ifaddr *ifa) > diff --git a/net/ipv4/arp.c b/net/ipv4/arp.c > index 7808093cede6..fc5ebc7e1962 100644 > --- a/net/ipv4/arp.c > +++ b/net/ipv4/arp.c > @@ -408,7 +408,8 @@ static int arp_ignore(struct in_device *in_dev, __be32 sip, __be32 tip) > default: > return 0; > } > - return !inet_confirm_addr(in_dev, sip, tip, scope); > + return !inet_confirm_addr(dev_net(in_dev->dev), in_dev, sip, tip, > + scope); We will need initial struct net *net = dev_net(in_dev->dev) because later we will set in_dev to NULL for arp_ignore=3. Then we will provide 'net' to inet_confirm_addr. > } > > static int arp_filter(__be32 sip, __be32 tip, struct net_device *dev) > diff --git a/net/ipv4/devinet.c b/net/ipv4/devinet.c > index a1b5bcbd04ae..47cdb035f817 100644 > --- a/net/ipv4/devinet.c > +++ b/net/ipv4/devinet.c > @@ -1236,22 +1236,22 @@ static __be32 confirm_addr_indev(struct in_device *in_dev, __be32 dst, > > /* > * Confirm that local IP address exists using wildcards: > + * - net: netns to check, cannot be NULL > * - in_dev: only on this interface, 0=any interface NULL is better than 0 > * - dst: only in the same subnet as dst, 0=any dst > * - local: address, 0=autoselect the local address > * - scope: maximum allowed scope value for the local address > */ > -__be32 inet_confirm_addr(struct in_device *in_dev, > +__be32 inet_confirm_addr(struct net *net, struct in_device *in_dev, > __be32 dst, __be32 local, int scope) > { > __be32 addr = 0; > struct net_device *dev; > - struct net *net; > > if (scope != RT_SCOPE_LINK) Use if (in_dev) > - return confirm_addr_indev(in_dev, dst, local, scope); > + return in_dev ? confirm_addr_indev(in_dev, dst, local, scope) : > + 0; We will not need the above change. > - net = dev_net(in_dev->dev); > rcu_read_lock(); > for_each_netdev_rcu(net, dev) { > in_dev = __in_dev_get_rcu(dev); > -- > 1.8.4.1 Regards -- Julian Anastasov <ja@ssi.bg> ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH net v2] ipv4: fix wildcard search with inet_confirm_addr() 2013-11-08 21:24 ` Julian Anastasov @ 2013-11-13 15:23 ` Nicolas Dichtel 2013-11-13 21:27 ` Julian Anastasov 0 siblings, 1 reply; 7+ messages in thread From: Nicolas Dichtel @ 2013-11-13 15:23 UTC (permalink / raw) To: ja; +Cc: davem, netdev, Nicolas Dichtel Help of this function says: "in_dev: only on this interface, 0=any interface", but since commit 39a6d0630012 ("[NETNS]: Process inet_confirm_addr in the correct namespace."), the code supposes that it will never be NULL. This function is never called with in_dev == NULL, but it's exported and may be used by an external module. Because this patch restore the ability to call inet_confirm_addr() with in_dev == NULL, I partially revert the above commit, as suggested by Julian. CC: Julian Anastasov <ja@ssi.bg> Signed-off-by: Nicolas Dichtel <nicolas.dichtel@6wind.com> --- This bug was spotted by code review. v2: fix change in bond_confirm_addr() partially revert 39a6d0630012 ("[NETNS]: Process inet_confirm_addr in the correct namespace.") fix API desc of inet_confirm_addr() drivers/net/bonding/bonding.h | 4 ++-- include/linux/inetdevice.h | 3 ++- net/ipv4/arp.c | 4 +++- net/ipv4/devinet.c | 9 ++++----- 4 files changed, 11 insertions(+), 9 deletions(-) diff --git a/drivers/net/bonding/bonding.h b/drivers/net/bonding/bonding.h index 03cf3fd14490..0ad3f4bd99c0 100644 --- a/drivers/net/bonding/bonding.h +++ b/drivers/net/bonding/bonding.h @@ -414,8 +414,8 @@ static inline __be32 bond_confirm_addr(struct net_device *dev, __be32 dst, __be3 in_dev = __in_dev_get_rcu(dev); if (in_dev) - addr = inet_confirm_addr(in_dev, dst, local, RT_SCOPE_HOST); - + addr = inet_confirm_addr(dev_net(dev), in_dev, dst, local, + RT_SCOPE_HOST); rcu_read_unlock(); return addr; } diff --git a/include/linux/inetdevice.h b/include/linux/inetdevice.h index 79640e015a86..5870f7060917 100644 --- a/include/linux/inetdevice.h +++ b/include/linux/inetdevice.h @@ -164,7 +164,8 @@ extern int devinet_ioctl(struct net *net, unsigned int cmd, void __user *); extern void devinet_init(void); extern struct in_device *inetdev_by_index(struct net *, int); extern __be32 inet_select_addr(const struct net_device *dev, __be32 dst, int scope); -extern __be32 inet_confirm_addr(struct in_device *in_dev, __be32 dst, __be32 local, int scope); +__be32 inet_confirm_addr(struct net *net, struct in_device *in_dev, __be32 dst, + __be32 local, int scope); extern struct in_ifaddr *inet_ifa_byprefix(struct in_device *in_dev, __be32 prefix, __be32 mask); static __inline__ int inet_ifa_match(__be32 addr, struct in_ifaddr *ifa) diff --git a/net/ipv4/arp.c b/net/ipv4/arp.c index 7808093cede6..46c7b4483bcf 100644 --- a/net/ipv4/arp.c +++ b/net/ipv4/arp.c @@ -379,6 +379,7 @@ static void arp_solicit(struct neighbour *neigh, struct sk_buff *skb) static int arp_ignore(struct in_device *in_dev, __be32 sip, __be32 tip) { + struct net *net = dev_net(in_dev->dev); int scope; switch (IN_DEV_ARP_IGNORE(in_dev)) { @@ -397,6 +398,7 @@ static int arp_ignore(struct in_device *in_dev, __be32 sip, __be32 tip) case 3: /* Do not reply for scope host addresses */ sip = 0; scope = RT_SCOPE_LINK; + in_dev = NULL; break; case 4: /* Reserved */ case 5: @@ -408,7 +410,7 @@ static int arp_ignore(struct in_device *in_dev, __be32 sip, __be32 tip) default: return 0; } - return !inet_confirm_addr(in_dev, sip, tip, scope); + return !inet_confirm_addr(net, in_dev, sip, tip, scope); } static int arp_filter(__be32 sip, __be32 tip, struct net_device *dev) diff --git a/net/ipv4/devinet.c b/net/ipv4/devinet.c index a1b5bcbd04ae..19426e4b232c 100644 --- a/net/ipv4/devinet.c +++ b/net/ipv4/devinet.c @@ -1236,22 +1236,21 @@ static __be32 confirm_addr_indev(struct in_device *in_dev, __be32 dst, /* * Confirm that local IP address exists using wildcards: - * - in_dev: only on this interface, 0=any interface + * - net: netns to check, cannot be NULL + * - in_dev: only on this interface, NULL=any interface * - dst: only in the same subnet as dst, 0=any dst * - local: address, 0=autoselect the local address * - scope: maximum allowed scope value for the local address */ -__be32 inet_confirm_addr(struct in_device *in_dev, +__be32 inet_confirm_addr(struct net *net, struct in_device *in_dev, __be32 dst, __be32 local, int scope) { __be32 addr = 0; struct net_device *dev; - struct net *net; - if (scope != RT_SCOPE_LINK) + if (in_dev != NULL) return confirm_addr_indev(in_dev, dst, local, scope); - net = dev_net(in_dev->dev); rcu_read_lock(); for_each_netdev_rcu(net, dev) { in_dev = __in_dev_get_rcu(dev); -- 1.8.4.1 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH net v2] ipv4: fix wildcard search with inet_confirm_addr() 2013-11-13 15:23 ` [PATCH net v2] " Nicolas Dichtel @ 2013-11-13 21:27 ` Julian Anastasov 2013-11-14 10:32 ` Nicolas Dichtel 2013-12-10 14:02 ` [PATCH net-next v3] " Nicolas Dichtel 0 siblings, 2 replies; 7+ messages in thread From: Julian Anastasov @ 2013-11-13 21:27 UTC (permalink / raw) To: Nicolas Dichtel; +Cc: davem, netdev Hello, On Wed, 13 Nov 2013, Nicolas Dichtel wrote: > Help of this function says: "in_dev: only on this interface, 0=any interface", > but since commit 39a6d0630012 ("[NETNS]: Process inet_confirm_addr in the > correct namespace."), the code supposes that it will never be NULL. This > function is never called with in_dev == NULL, but it's exported and may be used > by an external module. > > Because this patch restore the ability to call inet_confirm_addr() with in_dev > == NULL, I partially revert the above commit, as suggested by Julian. > > CC: Julian Anastasov <ja@ssi.bg> > Signed-off-by: Nicolas Dichtel <nicolas.dichtel@6wind.com> This patch looks ok to me, thanks! Reviewed-by: Julian Anastasov <ja@ssi.bg> Still, I think this is a net-next material and you have to wait some days... The net tree that you are using is missing the changes that remove 'extern', so I think you have to port it to net-next tree to avoid the conflict in include/linux/inetdevice.h. > --- > > This bug was spotted by code review. > > v2: fix change in bond_confirm_addr() > partially revert 39a6d0630012 ("[NETNS]: Process inet_confirm_addr in the > correct namespace.") > fix API desc of inet_confirm_addr() > > drivers/net/bonding/bonding.h | 4 ++-- > include/linux/inetdevice.h | 3 ++- > net/ipv4/arp.c | 4 +++- > net/ipv4/devinet.c | 9 ++++----- > 4 files changed, 11 insertions(+), 9 deletions(-) > > diff --git a/drivers/net/bonding/bonding.h b/drivers/net/bonding/bonding.h > index 03cf3fd14490..0ad3f4bd99c0 100644 > --- a/drivers/net/bonding/bonding.h > +++ b/drivers/net/bonding/bonding.h > @@ -414,8 +414,8 @@ static inline __be32 bond_confirm_addr(struct net_device *dev, __be32 dst, __be3 > in_dev = __in_dev_get_rcu(dev); > > if (in_dev) > - addr = inet_confirm_addr(in_dev, dst, local, RT_SCOPE_HOST); > - > + addr = inet_confirm_addr(dev_net(dev), in_dev, dst, local, > + RT_SCOPE_HOST); > rcu_read_unlock(); > return addr; > } > diff --git a/include/linux/inetdevice.h b/include/linux/inetdevice.h > index 79640e015a86..5870f7060917 100644 > --- a/include/linux/inetdevice.h > +++ b/include/linux/inetdevice.h > @@ -164,7 +164,8 @@ extern int devinet_ioctl(struct net *net, unsigned int cmd, void __user *); > extern void devinet_init(void); > extern struct in_device *inetdev_by_index(struct net *, int); > extern __be32 inet_select_addr(const struct net_device *dev, __be32 dst, int scope); > -extern __be32 inet_confirm_addr(struct in_device *in_dev, __be32 dst, __be32 local, int scope); > +__be32 inet_confirm_addr(struct net *net, struct in_device *in_dev, __be32 dst, > + __be32 local, int scope); > extern struct in_ifaddr *inet_ifa_byprefix(struct in_device *in_dev, __be32 prefix, __be32 mask); > > static __inline__ int inet_ifa_match(__be32 addr, struct in_ifaddr *ifa) > diff --git a/net/ipv4/arp.c b/net/ipv4/arp.c > index 7808093cede6..46c7b4483bcf 100644 > --- a/net/ipv4/arp.c > +++ b/net/ipv4/arp.c > @@ -379,6 +379,7 @@ static void arp_solicit(struct neighbour *neigh, struct sk_buff *skb) > > static int arp_ignore(struct in_device *in_dev, __be32 sip, __be32 tip) > { > + struct net *net = dev_net(in_dev->dev); > int scope; > > switch (IN_DEV_ARP_IGNORE(in_dev)) { > @@ -397,6 +398,7 @@ static int arp_ignore(struct in_device *in_dev, __be32 sip, __be32 tip) > case 3: /* Do not reply for scope host addresses */ > sip = 0; > scope = RT_SCOPE_LINK; > + in_dev = NULL; > break; > case 4: /* Reserved */ > case 5: > @@ -408,7 +410,7 @@ static int arp_ignore(struct in_device *in_dev, __be32 sip, __be32 tip) > default: > return 0; > } > - return !inet_confirm_addr(in_dev, sip, tip, scope); > + return !inet_confirm_addr(net, in_dev, sip, tip, scope); > } > > static int arp_filter(__be32 sip, __be32 tip, struct net_device *dev) > diff --git a/net/ipv4/devinet.c b/net/ipv4/devinet.c > index a1b5bcbd04ae..19426e4b232c 100644 > --- a/net/ipv4/devinet.c > +++ b/net/ipv4/devinet.c > @@ -1236,22 +1236,21 @@ static __be32 confirm_addr_indev(struct in_device *in_dev, __be32 dst, > > /* > * Confirm that local IP address exists using wildcards: > - * - in_dev: only on this interface, 0=any interface > + * - net: netns to check, cannot be NULL > + * - in_dev: only on this interface, NULL=any interface > * - dst: only in the same subnet as dst, 0=any dst > * - local: address, 0=autoselect the local address > * - scope: maximum allowed scope value for the local address > */ > -__be32 inet_confirm_addr(struct in_device *in_dev, > +__be32 inet_confirm_addr(struct net *net, struct in_device *in_dev, > __be32 dst, __be32 local, int scope) > { > __be32 addr = 0; > struct net_device *dev; > - struct net *net; > > - if (scope != RT_SCOPE_LINK) > + if (in_dev != NULL) > return confirm_addr_indev(in_dev, dst, local, scope); > > - net = dev_net(in_dev->dev); > rcu_read_lock(); > for_each_netdev_rcu(net, dev) { > in_dev = __in_dev_get_rcu(dev); > -- > 1.8.4.1 Regards -- Julian Anastasov <ja@ssi.bg> ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH net v2] ipv4: fix wildcard search with inet_confirm_addr() 2013-11-13 21:27 ` Julian Anastasov @ 2013-11-14 10:32 ` Nicolas Dichtel 2013-12-10 14:02 ` [PATCH net-next v3] " Nicolas Dichtel 1 sibling, 0 replies; 7+ messages in thread From: Nicolas Dichtel @ 2013-11-14 10:32 UTC (permalink / raw) To: Julian Anastasov; +Cc: davem, netdev Le 13/11/2013 22:27, Julian Anastasov a écrit : > > Hello, > > On Wed, 13 Nov 2013, Nicolas Dichtel wrote: > >> Help of this function says: "in_dev: only on this interface, 0=any interface", >> but since commit 39a6d0630012 ("[NETNS]: Process inet_confirm_addr in the >> correct namespace."), the code supposes that it will never be NULL. This >> function is never called with in_dev == NULL, but it's exported and may be used >> by an external module. >> >> Because this patch restore the ability to call inet_confirm_addr() with in_dev >> == NULL, I partially revert the above commit, as suggested by Julian. >> >> CC: Julian Anastasov <ja@ssi.bg> >> Signed-off-by: Nicolas Dichtel <nicolas.dichtel@6wind.com> > > This patch looks ok to me, thanks! > > Reviewed-by: Julian Anastasov <ja@ssi.bg> > > Still, I think this is a net-next material > and you have to wait some days... The net tree that Ok. > you are using is missing the changes that remove > 'extern', so I think you have to port it to net-next > tree to avoid the conflict in include/linux/inetdevice.h. Yes, checkpatch already warns about this extern kerword. Thank you, Nicolas ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH net-next v3] ipv4: fix wildcard search with inet_confirm_addr() 2013-11-13 21:27 ` Julian Anastasov 2013-11-14 10:32 ` Nicolas Dichtel @ 2013-12-10 14:02 ` Nicolas Dichtel 2013-12-11 19:49 ` David Miller 1 sibling, 1 reply; 7+ messages in thread From: Nicolas Dichtel @ 2013-12-10 14:02 UTC (permalink / raw) To: ja; +Cc: davem, netdev, Nicolas Dichtel Help of this function says: "in_dev: only on this interface, 0=any interface", but since commit 39a6d0630012 ("[NETNS]: Process inet_confirm_addr in the correct namespace."), the code supposes that it will never be NULL. This function is never called with in_dev == NULL, but it's exported and may be used by an external module. Because this patch restore the ability to call inet_confirm_addr() with in_dev == NULL, I partially revert the above commit, as suggested by Julian. CC: Julian Anastasov <ja@ssi.bg> Signed-off-by: Nicolas Dichtel <nicolas.dichtel@6wind.com> Reviewed-by: Julian Anastasov <ja@ssi.bg> --- This was spotted by code review. v3: rebase patch on net-next v2: fix change in bond_confirm_addr() partially revert 39a6d0630012 ("[NETNS]: Process inet_confirm_addr in the correct namespace.") fix API desc of inet_confirm_addr() drivers/net/bonding/bonding.h | 4 ++-- include/linux/inetdevice.h | 5 ++--- net/ipv4/arp.c | 4 +++- net/ipv4/devinet.c | 9 ++++----- 4 files changed, 11 insertions(+), 11 deletions(-) diff --git a/drivers/net/bonding/bonding.h b/drivers/net/bonding/bonding.h index a9f4f9f4d8ce..a74c92c83ead 100644 --- a/drivers/net/bonding/bonding.h +++ b/drivers/net/bonding/bonding.h @@ -394,8 +394,8 @@ static inline __be32 bond_confirm_addr(struct net_device *dev, __be32 dst, __be3 in_dev = __in_dev_get_rcu(dev); if (in_dev) - addr = inet_confirm_addr(in_dev, dst, local, RT_SCOPE_HOST); - + addr = inet_confirm_addr(dev_net(dev), in_dev, dst, local, + RT_SCOPE_HOST); rcu_read_unlock(); return addr; } diff --git a/include/linux/inetdevice.h b/include/linux/inetdevice.h index ae174ca565c9..72edb60e0072 100644 --- a/include/linux/inetdevice.h +++ b/include/linux/inetdevice.h @@ -164,11 +164,10 @@ int devinet_ioctl(struct net *net, unsigned int cmd, void __user *); void devinet_init(void); struct in_device *inetdev_by_index(struct net *, int); __be32 inet_select_addr(const struct net_device *dev, __be32 dst, int scope); -__be32 inet_confirm_addr(struct in_device *in_dev, __be32 dst, __be32 local, - int scope); +__be32 inet_confirm_addr(struct net *net, struct in_device *in_dev, __be32 dst, + __be32 local, int scope); struct in_ifaddr *inet_ifa_byprefix(struct in_device *in_dev, __be32 prefix, __be32 mask); - static __inline__ int inet_ifa_match(__be32 addr, struct in_ifaddr *ifa) { return !((addr^ifa->ifa_address)&ifa->ifa_mask); diff --git a/net/ipv4/arp.c b/net/ipv4/arp.c index f04185863940..b67cf805910e 100644 --- a/net/ipv4/arp.c +++ b/net/ipv4/arp.c @@ -381,6 +381,7 @@ static void arp_solicit(struct neighbour *neigh, struct sk_buff *skb) static int arp_ignore(struct in_device *in_dev, __be32 sip, __be32 tip) { + struct net *net = dev_net(in_dev->dev); int scope; switch (IN_DEV_ARP_IGNORE(in_dev)) { @@ -399,6 +400,7 @@ static int arp_ignore(struct in_device *in_dev, __be32 sip, __be32 tip) case 3: /* Do not reply for scope host addresses */ sip = 0; scope = RT_SCOPE_LINK; + in_dev = NULL; break; case 4: /* Reserved */ case 5: @@ -410,7 +412,7 @@ static int arp_ignore(struct in_device *in_dev, __be32 sip, __be32 tip) default: return 0; } - return !inet_confirm_addr(in_dev, sip, tip, scope); + return !inet_confirm_addr(net, in_dev, sip, tip, scope); } static int arp_filter(__be32 sip, __be32 tip, struct net_device *dev) diff --git a/net/ipv4/devinet.c b/net/ipv4/devinet.c index 43065be36301..7316ad1dfda5 100644 --- a/net/ipv4/devinet.c +++ b/net/ipv4/devinet.c @@ -1238,22 +1238,21 @@ static __be32 confirm_addr_indev(struct in_device *in_dev, __be32 dst, /* * Confirm that local IP address exists using wildcards: - * - in_dev: only on this interface, 0=any interface + * - net: netns to check, cannot be NULL + * - in_dev: only on this interface, NULL=any interface * - dst: only in the same subnet as dst, 0=any dst * - local: address, 0=autoselect the local address * - scope: maximum allowed scope value for the local address */ -__be32 inet_confirm_addr(struct in_device *in_dev, +__be32 inet_confirm_addr(struct net *net, struct in_device *in_dev, __be32 dst, __be32 local, int scope) { __be32 addr = 0; struct net_device *dev; - struct net *net; - if (scope != RT_SCOPE_LINK) + if (in_dev != NULL) return confirm_addr_indev(in_dev, dst, local, scope); - net = dev_net(in_dev->dev); rcu_read_lock(); for_each_netdev_rcu(net, dev) { in_dev = __in_dev_get_rcu(dev); -- 1.8.4.1 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH net-next v3] ipv4: fix wildcard search with inet_confirm_addr() 2013-12-10 14:02 ` [PATCH net-next v3] " Nicolas Dichtel @ 2013-12-11 19:49 ` David Miller 0 siblings, 0 replies; 7+ messages in thread From: David Miller @ 2013-12-11 19:49 UTC (permalink / raw) To: nicolas.dichtel; +Cc: ja, netdev From: Nicolas Dichtel <nicolas.dichtel@6wind.com> Date: Tue, 10 Dec 2013 15:02:40 +0100 > 39a6d0630012 ("[NETNS]: Process inet_confirm_addr in the > correct namespace."), the code supposes that it will never be NULL. This > function is never called with in_dev == NULL, but it's exported and may be used > by an external module. > > Because this patch restore the ability to call inet_confirm_addr() with in_dev > == NULL, I partially revert the above commit, as suggested by Julian. > > CC: Julian Anastasov <ja@ssi.bg> > Signed-off-by: Nicolas Dichtel <nicolas.dichtel@6wind.com> > Reviewed-by: Julian Anastasov <ja@ssi.bg> I personally think that we may be over analyzing this, because if some external module actually cared, someone would have complained in the last _5_ years as that's how long the new semantics have been in place. Nevertheless, passing in the network namespace explicitly is a bit nicer than having special behavior for RT_SCOPE_LINE, thus I've applied this. Thanks. ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2013-12-11 19:49 UTC | newest] Thread overview: 7+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2013-11-08 14:37 [PATCH net] ipv4: fix wildcard search with inet_confirm_addr() Nicolas Dichtel 2013-11-08 21:24 ` Julian Anastasov 2013-11-13 15:23 ` [PATCH net v2] " Nicolas Dichtel 2013-11-13 21:27 ` Julian Anastasov 2013-11-14 10:32 ` Nicolas Dichtel 2013-12-10 14:02 ` [PATCH net-next v3] " Nicolas Dichtel 2013-12-11 19:49 ` David Miller
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).