This is an automated email from the ASF dual-hosted git repository.
archer pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/incubator-nuttx.git
The following commit(s) were added to refs/heads/master by this push:
new 7332d2d net/: Add missing packet filtering checks
7332d2d is described below
commit 7332d2decf819cecc172999de1d8c0d32c18c4e9
Author: Gregory Nutt <[email protected]>
AuthorDate: Wed Apr 28 14:18:16 2021 -0600
net/: Add missing packet filtering checks
NuttX provides the UDP_BINDTODEVICE socket option. This is a UDP
protocol-specific implementation of the semi-standard Linux SO_BINDTODEVICE
socket option: "SO_BINDTODEVICE forces packets on the socket to only egress
the bound interface, regardless of what the IP routing table would normally
choose. Similarly only packets which ingress the bound interface will be
received on the socket, packets from other interfaces will not be delivered to
the socket." https://codingrelic.geekhold. [...]
If CONFIG_NET_UDP_BINDTODEVICE is selected and a UDP socket is bound to the
device, then unrecognized packets UDP packets must not be dropped, but must be
forwarded along to the bound socket unconditionally.
It the typical case, this should have no impact. It does effect the
applications that use DHCP and do select the UDP_BINDTODEVICE socket option.
This PR replace existing improper logic in the code and also the improper
attempts to fix problems from PR #3601 and PR #3598. Those changes are
improper because they expose DHCP appliction dependencies in the OS, breaking
modularity and independence of the OS and application.
Tested with stm32f4discovery:netnsh with CONFIG_NET_UDP_BINDTODEVICE. A
proper DHCP test setup is needed, however.
---
include/net/if.h | 6 ++-
net/devif/ipv4_input.c | 9 ++++-
net/devif/ipv6_input.c | 9 ++++-
net/udp/udp_close.c | 28 +++++++++++++-
net/udp/udp_setsockopt.c | 95 +++++++++++++++++++++++++++++++++++-------------
5 files changed, 118 insertions(+), 29 deletions(-)
diff --git a/include/net/if.h b/include/net/if.h
index 44f467a..f68f9e9 100644
--- a/include/net/if.h
+++ b/include/net/if.h
@@ -44,6 +44,7 @@
#define IFF_UP (1 << 1) /* Interface is up */
#define IFF_RUNNING (1 << 2) /* Carrier is available */
#define IFF_IPv6 (1 << 3) /* Configured for IPv6 packet (vs ARP or
IPv4) */
+#define IFF_BOUND (1 << 4) /* Bound to a socket */
#define IFF_NOARP (1 << 7) /* ARP is not required for this packet */
/* Interface flag helpers */
@@ -51,20 +52,23 @@
#define IFF_SET_DOWN(f) do { (f) |= IFF_DOWN; } while (0)
#define IFF_SET_UP(f) do { (f) |= IFF_UP; } while (0)
#define IFF_SET_RUNNING(f) do { (f) |= IFF_RUNNING; } while (0)
+#define IFF_SET_BOUND(f) do { (f) |= IFF_BOUND; } while (0)
#define IFF_SET_NOARP(f) do { (f) |= IFF_NOARP; } while (0)
#define IFF_CLR_DOWN(f) do { (f) &= ~IFF_DOWN; } while (0)
#define IFF_CLR_UP(f) do { (f) &= ~IFF_UP; } while (0)
#define IFF_CLR_RUNNING(f) do { (f) &= ~IFF_RUNNING; } while (0)
+#define IFF_CLR_BOUND(f) do { (f) &= ~IFF_BOUND; } while (0)
#define IFF_CLR_NOARP(f) do { (f) &= ~IFF_NOARP; } while (0)
#define IFF_IS_DOWN(f) (((f) & IFF_DOWN) != 0)
#define IFF_IS_UP(f) (((f) & IFF_UP) != 0)
#define IFF_IS_RUNNING(f) (((f) & IFF_RUNNING) != 0)
+#define IFF_IS_BOUND(f) (((f) & IFF_BOUND) != 0)
#define IFF_IS_NOARP(f) (((f) & IFF_NOARP) != 0)
/* We only need to manage the IPv6 bit if both IPv6 and IPv4 are supported.
- * Otherwise, we can save a few bytes by ignoring it.
+ * Otherwise, we can save a few bytes by ignoring it.
*/
#if defined(CONFIG_NET_IPv4) && defined(CONFIG_NET_IPv6)
diff --git a/net/devif/ipv4_input.c b/net/devif/ipv4_input.c
index e0227a1..fa45b93 100644
--- a/net/devif/ipv4_input.c
+++ b/net/devif/ipv4_input.c
@@ -297,7 +297,14 @@ int ipv4_input(FAR struct net_driver_s *dev)
}
else
#endif
- if (ipv4->proto != IP_PROTO_UDP)
+#if defined(NET_UDP_HAVE_STACK) && defined(CONFIG_NET_UDP_BINDTODEVICE)
+ /* If the UDP protocol specific socket option UDP_BINDTODEVICE
+ * is selected, then we must forward all UDP packets to the bound
+ * socket.
+ */
+
+ if (ipv4->proto != IP_PROTO_UDP || !IFF_IS_BOUND(dev->d_flags))
+#endif
{
/* Not destined for us and not forwardable... Drop the
* packet.
diff --git a/net/devif/ipv6_input.c b/net/devif/ipv6_input.c
index 168cffd..2c42602 100644
--- a/net/devif/ipv6_input.c
+++ b/net/devif/ipv6_input.c
@@ -433,7 +433,14 @@ int ipv6_input(FAR struct net_driver_s *dev)
}
else
#endif
- if (nxthdr != IP_PROTO_UDP)
+#if defined(NET_UDP_HAVE_STACK) && defined(CONFIG_NET_UDP_BINDTODEVICE)
+ /* If the UDP protocol specific socket option UDP_BINDTODEVICE
+ * is selected, then we must forward all UDP packets to the bound
+ * socket.
+ */
+
+ if (nxthdr != IP_PROTO_UDP || !IFF_IS_BOUND(dev->d_flags))
+#endif
{
/* Not destined for us and not forwardable...
* drop the packet.
diff --git a/net/udp/udp_close.c b/net/udp/udp_close.c
index 2e4722e..b2610ce 100644
--- a/net/udp/udp_close.c
+++ b/net/udp/udp_close.c
@@ -29,10 +29,14 @@
#include <debug.h>
#include <assert.h>
+#include <net/if.h>
+
#include <nuttx/net/net.h>
+#include <nuttx/net/netdev.h>
#include <nuttx/net/udp.h>
#include "devif/devif.h"
+#include "netdev/netdev.h"
#include "udp/udp.h"
#include "socket/socket.h"
@@ -63,7 +67,7 @@ int udp_close(FAR struct socket *psock)
unsigned int timeout = UINT_MAX;
int ret;
- /* Interrupts are disabled here to avoid race conditions */
+ /* Lock the network to avoid race conditions */
net_lock();
@@ -102,6 +106,28 @@ int udp_close(FAR struct socket *psock)
nerr("ERROR: udp_txdrain() failed: %d\n", ret);
}
+#ifdef CONFIG_NET_UDP_BINDTODEVICE
+ /* Is the socket bound to an interface device */
+
+ if (conn->boundto != 0)
+ {
+ FAR struct net_driver_s *dev;
+
+ /* Yes, get the interface that we are bound do. NULL would indicate
+ * that the interface no longer exists for some reason.
+ */
+
+ dev = netdev_findbyindex(conn->boundto);
+ if (dev != NULL)
+ {
+ /* Clear the interface flag to unbind the device from the socket.
+ */
+
+ IFF_CLR_BOUND(dev->d_flags);
+ }
+ }
+#endif
+
#ifdef CONFIG_NET_UDP_WRITE_BUFFERS
/* Free any semi-permanent write buffer callback in place. */
diff --git a/net/udp/udp_setsockopt.c b/net/udp/udp_setsockopt.c
index 880b4d9..bd5bd99 100644
--- a/net/udp/udp_setsockopt.c
+++ b/net/udp/udp_setsockopt.c
@@ -29,9 +29,11 @@
#include <assert.h>
#include <debug.h>
+#include <net/if.h>
#include <netinet/udp.h>
#include <nuttx/net/net.h>
+#include <nuttx/net/netdev.h>
#include <nuttx/net/udp.h>
#include "socket/socket.h"
@@ -112,31 +114,74 @@ int udp_setsockopt(FAR struct socket *psock, int option,
*/
case UDP_BINDTODEVICE: /* Bind socket to a specific network device */
- if (value == NULL || value_len == 0 ||
- (value_len > 0 && ((FAR char *)value)[0] == 0))
- {
- conn->boundto = 0; /* This interface is no longer bound */
- ret = OK;
- }
- else
- {
- int ifindex;
-
- /* Get the interface index corresponding to the interface name */
-
- ifindex = netdev_nametoindex(value);
- if (ifindex >= 0)
- {
- DEBUGASSERT(ifindex > 0 && ifindex <= MAX_IFINDEX);
- conn->boundto = ifindex;
- ret = OK;
- }
- else
- {
- ret = ifindex;
- }
- }
-
+ {
+ FAR struct net_driver_s *dev;
+
+ /* Check if we are are unbinding the socket */
+
+ if (value == NULL || value_len == 0 ||
+ (value_len > 0 && ((FAR char *)value)[0] == 0))
+ {
+ /* Just report success if the socket is not bound to an
+ * interface.
+ */
+
+ if (conn->boundto != 0)
+ {
+ /* Get the interface that we are bound do. NULL would
+ * indicate that the interface no longer exists for some
+ * reason.
+ */
+
+ dev = netdev_findbyindex(conn->boundto);
+ if (dev != NULL)
+ {
+ /* Clear the interface flag to unbind the device from
+ * the socket.
+ */
+
+ IFF_CLR_BOUND(dev->d_flags);
+ }
+
+ conn->boundto = 0; /* This interface is no longer bound */
+ }
+
+ ret = OK;
+ }
+
+ /* No, we are binding a socket to the interface. */
+
+ else
+ {
+ /* Find the interface device with this name */
+
+ dev = netdev_findbyname(value);
+ if (dev == NULL)
+ {
+ ret = -ENODEV;
+ }
+
+ /* An interface may be bound only to one socket. */
+
+ else if (IFF_IS_BOUND(dev->d_flags))
+ {
+ ret = -EBUSY;
+ }
+ else
+ {
+ /* Bind the interface to a socket */
+
+ IFF_SET_BOUND(dev->d_flags);
+
+ /* Bind the socket to the interface */
+
+ DEBUGASSERT(dev->d_ifindex > 0 &&
+ dev->d_ifindex <= MAX_IFINDEX);
+ conn->boundto = dev->d_ifindex;
+ ret = OK;
+ }
+ }
+ }
break;
#endif
