[PATCH 1/9] utils/can: prevent unterminated destination buffer with strncpy()

[PATCH 1/9] utils/can: prevent unterminated destination buffer with strncpy()

[Philippe Gerum]

GCC 8.x introduced -Wstringop-truncation to help detecting likely
unwanted outcomes of strncpy(dst, src, n), such as omitting the NUL
character into the destination buffer whenever n < sizeof(src).

Fix unsafe strncpy() calls when we do expect a null-terminated
destination buffer.
---
 utils/can/rtcanconfig.c | 7 ++++---
 utils/can/rtcanrecv.c   | 3 ++-
 utils/can/rtcansend.c   | 3 ++-
 3 files changed, 8 insertions(+), 5 deletions(-)

diff --git a/utils/can/rtcanconfig.c b/utils/can/rtcanconfig.c
index a285ec73d..395e0368d 100644
--- a/utils/can/rtcanconfig.c
+++ b/utils/can/rtcanconfig.c
@@ -31,6 +31,7 @@
 #include <errno.h>
 #include <getopt.h>
 #include <sys/mman.h>
+#include <boilerplate/ancillaries.h>
 
 #include <rtdm/can.h>
 
@@ -81,7 +82,7 @@ static int string_to_ctrlmode(char *str)
 
 int main(int argc, char *argv[])
 {
-    char    ifname[16];
+    char    ifname[IFNAMSIZ];
     int     can_fd = -1;
     int     new_baudrate = -1;
     int     new_mode = -1;
@@ -159,8 +160,8 @@ int main(int argc, char *argv[])
 	return 0;
     }
 
-    strncpy(ifname, argv[optind], IFNAMSIZ);
-    strncpy(ifr.ifr_name, ifname, IFNAMSIZ);
+    namecpy(ifname, argv[optind]);
+    namecpy(ifr.ifr_name, ifname);
 
     if (optind == argc - 2) {   /* Get mode setting */
 	new_mode = string_to_mode(argv[optind + 1]);
diff --git a/utils/can/rtcanrecv.c b/utils/can/rtcanrecv.c
index 8155ab76d..71e68cc5c 100644
--- a/utils/can/rtcanrecv.c
+++ b/utils/can/rtcanrecv.c
@@ -7,6 +7,7 @@
 #include <getopt.h>
 
 #include <alchemy/task.h>
+#include <boilerplate/ancillaries.h>
 
 #include <rtdm/can.h>
 
@@ -248,7 +249,7 @@ int main(int argc, char **argv)
 	if (verbose)
 	    printf("interface %s\n", argv[optind]);
 
-	strncpy(ifr.ifr_name, argv[optind], IFNAMSIZ);
+	namecpy(ifr.ifr_name, argv[optind]);
 	if (verbose)
 	    printf("s=%d, ifr_name=%s\n", s, ifr.ifr_name);
 
diff --git a/utils/can/rtcansend.c b/utils/can/rtcansend.c
index 4a692b3bf..bfa3054c2 100644
--- a/utils/can/rtcansend.c
+++ b/utils/can/rtcansend.c
@@ -6,6 +6,7 @@
 #include <errno.h>
 #include <getopt.h>
 
+#include <boilerplate/ancillaries.h>
 #include <alchemy/task.h>
 #include <alchemy/timer.h>
 
@@ -231,7 +232,7 @@ int main(int argc, char **argv)
 	    printf("Using loopback=%d\n", loopback);
     }
 
-    strncpy(ifr.ifr_name, argv[optind], IFNAMSIZ);
+    namecpy(ifr.ifr_name, argv[optind]);
     if (verbose)
 	printf("s=%d, ifr_name=%s\n", s, ifr.ifr_name);
 
-- 
2.17.2

[PATCH 2/9] demos/posix: prevent unterminated destination buffer with strncpy()

[Philippe Gerum]

GCC 8.x introduced -Wstringop-truncation to help detecting likely
unwanted outcomes of strncpy(dst, src, n), such as omitting the NUL
character into the destination buffer whenever n < sizeof(src).

Fix unsafe strncpy() calls when we do expect a null-terminated
destination buffer.
---
 demo/posix/cobalt/can-rtt.c        | 4 ++--
 demo/posix/cobalt/eth_p_all.c      | 3 ++-
 demo/posix/cyclictest/cyclictest.c | 4 ++--
 3 files changed, 6 insertions(+), 5 deletions(-)

diff --git a/demo/posix/cobalt/can-rtt.c b/demo/posix/cobalt/can-rtt.c
index 61cad05e5..dd212d804 100644
--- a/demo/posix/cobalt/can-rtt.c
+++ b/demo/posix/cobalt/can-rtt.c
@@ -248,7 +248,7 @@ int main(int argc, char *argv[])
 	return -1;
     }
 
-    strncpy(ifr.ifr_name, rxdev, IFNAMSIZ);
+    namecpy(ifr.ifr_name, rxdev);
     printf("RX rxsock=%d, ifr_name=%s\n", rxsock, ifr.ifr_name);
 
     if (ioctl(rxsock, SIOCGIFINDEX, &ifr) < 0) {
@@ -282,7 +282,7 @@ int main(int argc, char *argv[])
 	    goto failure1;
 	}
 
-	strncpy(ifr.ifr_name, txdev, IFNAMSIZ);
+	namecpy(ifr.ifr_name, txdev);
 	printf("TX txsock=%d, ifr_name=%s\n", txsock, ifr.ifr_name);
 
 	if (ioctl(txsock, SIOCGIFINDEX, &ifr) < 0) {
diff --git a/demo/posix/cobalt/eth_p_all.c b/demo/posix/cobalt/eth_p_all.c
index 6ac12ab3e..91aef9fbd 100644
--- a/demo/posix/cobalt/eth_p_all.c
+++ b/demo/posix/cobalt/eth_p_all.c
@@ -40,6 +40,7 @@
 #include <net/if.h>
 #include <arpa/inet.h>
 #include <netinet/ether.h>
+#include <boilerplate/ancillaries.h>
 
 char buffer[10*1024];
 int sock;
@@ -72,7 +73,7 @@ int main(int argc, char *argv[])
 	if (argc > 1) {
 		struct ifreq ifr;
 
-		strncpy(ifr.ifr_name, argv[1], IFNAMSIZ);
+		namecpy(ifr.ifr_name, argv[1]);
 		if (ioctl(sock, SIOCGIFINDEX, &ifr) < 0) {
 			perror("cannot get interface index");
 			close(sock);
diff --git a/demo/posix/cyclictest/cyclictest.c b/demo/posix/cyclictest/cyclictest.c
index ebe5461db..76983bd02 100644
--- a/demo/posix/cyclictest/cyclictest.c
+++ b/demo/posix/cyclictest/cyclictest.c
@@ -1353,7 +1353,7 @@ static void process_options (int argc, char *argv[], int max_cpus)
 		case 'F':
 		case OPT_FIFO:
 			use_fifo = 1;
-			strncpy(fifopath, optarg, strlen(optarg));
+			strncpy(fifopath, optarg, sizeof(fifopath) - 1);
 			break;
 
 		case 'H':
@@ -1458,7 +1458,7 @@ static void process_options (int argc, char *argv[], int max_cpus)
 		case 'T':
 		case OPT_TRACER:
 			tracetype = CUSTOM;
-			strncpy(tracer, optarg, sizeof(tracer));
+			strncpy(tracer, optarg, sizeof(tracer) - 1);
 			break;
 		case 'u':
 		case OPT_UNBUFFERED:
-- 
2.17.2

[PATCH 3/9] boilerplate/ancillaries: prevent false positive with -Wstringop-truncation

[Philippe Gerum]

namecpy() is meant to copying NUL terminated strings even if this
incurs potential truncation of the source argument if need be, prevent
GCC 8.x from detecting a false positive when -Wstringop-truncation is
in effect.
---
 include/boilerplate/ancillaries.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/boilerplate/ancillaries.h b/include/boilerplate/ancillaries.h
index f3b55668c..319d22f66 100644
--- a/include/boilerplate/ancillaries.h
+++ b/include/boilerplate/ancillaries.h
@@ -34,7 +34,7 @@ void __namecpy_requires_character_array_as_destination(void);
 	({								\
 		if (!__builtin_types_compatible_p(typeof(__dst), char[])) \
 			__namecpy_requires_character_array_as_destination();	\
-		strncpy((__dst), __src, sizeof(__dst) - 1);		\
+		strncpy((__dst), __src, sizeof(__dst));			\
 		__dst[sizeof(__dst) - 1] = '\0';			\
 		__dst;							\
 	 })
-- 
2.17.2

[PATCH 4/9] cobalt/ancillaries: prevent false positive with -Wstringop-truncation

[Philippe Gerum]

knamecpy() is meant to copying NUL terminated strings even if this
incurs potential truncation of the source argument if need be, prevent
GCC 8.x from detecting a false positive when -Wstringop-truncation is
in effect.
---
 include/cobalt/kernel/ancillaries.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/cobalt/kernel/ancillaries.h b/include/cobalt/kernel/ancillaries.h
index cde9856da..b957310c7 100644
--- a/include/cobalt/kernel/ancillaries.h
+++ b/include/cobalt/kernel/ancillaries.h
@@ -58,7 +58,7 @@ void __knamecpy_requires_character_array_as_destination(void);
 	({								\
 		if (!__builtin_types_compatible_p(typeof(__dst), char[])) \
 			__knamecpy_requires_character_array_as_destination();	\
-		strncpy((__dst), __src, sizeof(__dst) - 1);		\
+		strncpy((__dst), __src, sizeof(__dst));			\
 		__dst[sizeof(__dst) - 1] = '\0';			\
 		__dst;							\
 	 })
-- 
2.17.2

[PATCH 5/9] boilerplate/compiler: drop __const and __pure shorthands

[Philippe Gerum]

We have no in-tree users for these anymore. Besides, __const may be
conflicting with different definitions from legacy libc headers.
---
 include/boilerplate/compiler.h | 8 --------
 1 file changed, 8 deletions(-)

diff --git a/include/boilerplate/compiler.h b/include/boilerplate/compiler.h
index f526eb269..ffeb6a4db 100644
--- a/include/boilerplate/compiler.h
+++ b/include/boilerplate/compiler.h
@@ -46,14 +46,6 @@
 #define __weak		__attribute__((__weak__))
 #endif
 
-#ifndef __const
-#define __const		__attribute__((__const__))
-#endif
-
-#ifndef __pure
-#define __pure		__attribute__((__pure__))
-#endif
-
 #ifndef __maybe_unused
 #define __maybe_unused	__attribute__((__unused__))
 #endif
-- 
2.17.2

[PATCH 6/9] net/stack: export services to switch interface up/down

[Philippe Gerum]

We may want to force a rtnet interface up, and more likely down before
unloading a driver module.

Export the ioctl() code which handles the IOC_RT_IFUP/IFDOWN core
ioctl requests as standalone routines. The locking model remains
unchanged, with ioctl() requests and direct service calls serializing
on the per-device lock.
---
 kernel/drivers/net/stack/include/rtdev.h |  4 +
 kernel/drivers/net/stack/rtdev.c         | 98 ++++++++++++++++++++++++
 kernel/drivers/net/stack/rtnet_chrdev.c  | 85 +-------------------
 3 files changed, 104 insertions(+), 83 deletions(-)

diff --git a/kernel/drivers/net/stack/include/rtdev.h b/kernel/drivers/net/stack/include/rtdev.h
index ad29cfaf2..ddc7aebcd 100644
--- a/kernel/drivers/net/stack/include/rtdev.h
+++ b/kernel/drivers/net/stack/include/rtdev.h
@@ -231,6 +231,10 @@ unsigned int rt_hard_mtu(struct rtnet_device *rtdev, unsigned int priority);
 int rtdev_open(struct rtnet_device *rtdev);
 int rtdev_close(struct rtnet_device *rtdev);
 
+int rtdev_up(struct rtnet_device *rtdev,
+	     struct rtnet_core_cmd *cmd);
+int rtdev_down(struct rtnet_device *rtdev);
+
 int rtdev_map_rtskb(struct rtskb *skb);
 void rtdev_unmap_rtskb(struct rtskb *skb);
 
diff --git a/kernel/drivers/net/stack/rtdev.c b/kernel/drivers/net/stack/rtdev.c
index ed6e55fcf..8f3460afd 100644
--- a/kernel/drivers/net/stack/rtdev.c
+++ b/kernel/drivers/net/stack/rtdev.c
@@ -650,7 +650,105 @@ void rtdev_del_event_hook(struct rtdev_event_hook *hook)
     mutex_unlock(&rtnet_devices_nrt_lock);
 }
 
+int rtdev_up(struct rtnet_device *rtdev, struct rtnet_core_cmd *cmd)
+{
+	struct list_head        *entry;
+	struct rtdev_event_hook *hook;
+	int ret = 0;
+
+	if (mutex_lock_interruptible(&rtdev->nrt_lock))
+		return -ERESTARTSYS;
+
+	/* We cannot change the promisc flag or the hardware address if
+	   the device is already up. */
+	if ((rtdev->flags & IFF_UP) &&
+	    (((cmd->args.up.set_dev_flags | cmd->args.up.clear_dev_flags) &
+	      IFF_PROMISC) ||
+	     (cmd->args.up.dev_addr_type != ARPHRD_VOID))) {
+		ret = -EBUSY;
+		goto up_out;
+	}
+
+	rtdev->flags |= cmd->args.up.set_dev_flags;
+	rtdev->flags &= ~cmd->args.up.clear_dev_flags;
+
+	if (cmd->args.up.dev_addr_type != ARPHRD_VOID) {
+		if (cmd->args.up.dev_addr_type != rtdev->type) {
+			ret = -EINVAL;
+			goto up_out;
+		}
+		memcpy(rtdev->dev_addr, cmd->args.up.dev_addr, MAX_ADDR_LEN);
+	}
+
+	set_bit(PRIV_FLAG_UP, &rtdev->priv_flags);
+
+	ret = rtdev_open(rtdev);    /* also == 0 if rtdev is already up */
+
+	if (ret == 0) {
+		mutex_lock(&rtnet_devices_nrt_lock);
+
+		list_for_each(entry, &event_hook_list) {
+			hook = list_entry(entry, struct rtdev_event_hook, entry);
+			if (hook->ifup)
+				hook->ifup(rtdev, cmd);
+		}
+
+		mutex_unlock(&rtnet_devices_nrt_lock);
+	} else
+		clear_bit(PRIV_FLAG_UP, &rtdev->priv_flags);
+
+up_out:
+	mutex_unlock(&rtdev->nrt_lock);
+
+	return ret;
+}
+EXPORT_SYMBOL_GPL(rtdev_up);
+
+int rtdev_down(struct rtnet_device *rtdev)
+{
+	struct list_head        *entry;
+	struct rtdev_event_hook *hook;
+	rtdm_lockctx_t          context;
+	int ret = 0;
 
+	if (mutex_lock_interruptible(&rtdev->nrt_lock))
+		return -ERESTARTSYS;
+
+	/* spin lock required for sync with routing code */
+	rtdm_lock_get_irqsave(&rtdev->rtdev_lock, context);
+
+	if (test_bit(PRIV_FLAG_ADDING_ROUTE, &rtdev->priv_flags)) {
+		rtdm_lock_put_irqrestore(&rtdev->rtdev_lock, context);
+
+		mutex_unlock(&rtdev->nrt_lock);
+		return -EBUSY;
+	}
+	clear_bit(PRIV_FLAG_UP, &rtdev->priv_flags);
+
+	rtdm_lock_put_irqrestore(&rtdev->rtdev_lock, context);
+
+	if (rtdev->mac_detach != NULL)
+		ret = rtdev->mac_detach(rtdev);
+
+	if (ret == 0) {
+		mutex_lock(&rtnet_devices_nrt_lock);
+
+		list_for_each(entry, &event_hook_list) {
+			hook = list_entry(entry, struct rtdev_event_hook, entry);
+			if (hook->ifdown)
+				hook->ifdown(rtdev);
+		}
+
+		mutex_unlock(&rtnet_devices_nrt_lock);
+
+		ret = rtdev_close(rtdev);
+	}
+
+	mutex_unlock(&rtdev->nrt_lock);
+
+	return ret;
+}
+EXPORT_SYMBOL_GPL(rtdev_down);
 
 /***
  *  rtdev_open
diff --git a/kernel/drivers/net/stack/rtnet_chrdev.c b/kernel/drivers/net/stack/rtnet_chrdev.c
index f5f32c978..14ccbabaf 100644
--- a/kernel/drivers/net/stack/rtnet_chrdev.c
+++ b/kernel/drivers/net/stack/rtnet_chrdev.c
@@ -88,16 +88,11 @@ static long rtnet_ioctl(struct file *file,
     return -ENOTTY;
 }
 
-
-
 static int rtnet_core_ioctl(struct rtnet_device *rtdev, unsigned int request,
 			    unsigned long arg)
 {
     struct rtnet_core_cmd   cmd;
-    struct list_head        *entry;
-    struct rtdev_event_hook *hook;
     int                     ret;
-    rtdm_lockctx_t          context;
 
 
     ret = copy_from_user(&cmd, (void *)arg, sizeof(cmd));
@@ -106,87 +101,11 @@ static int rtnet_core_ioctl(struct rtnet_device *rtdev, unsigned int request,
 
     switch (request) {
 	case IOC_RT_IFUP:
-	    if (mutex_lock_interruptible(&rtdev->nrt_lock))
-		return -ERESTARTSYS;
-
-	    /* We cannot change the promisc flag or the hardware address if
-	       the device is already up. */
-	    if ((rtdev->flags & IFF_UP) &&
-		(((cmd.args.up.set_dev_flags | cmd.args.up.clear_dev_flags) &
-		  IFF_PROMISC) ||
-		 (cmd.args.up.dev_addr_type != ARPHRD_VOID))) {
-		ret = -EBUSY;
-		goto up_out;
-	    }
-
-	    rtdev->flags |= cmd.args.up.set_dev_flags;
-	    rtdev->flags &= ~cmd.args.up.clear_dev_flags;
-
-	    if (cmd.args.up.dev_addr_type != ARPHRD_VOID) {
-		if (cmd.args.up.dev_addr_type != rtdev->type) {
-		    ret = -EINVAL;
-		    goto up_out;
-		}
-		memcpy(rtdev->dev_addr, cmd.args.up.dev_addr, MAX_ADDR_LEN);
-	    }
-
-	    set_bit(PRIV_FLAG_UP, &rtdev->priv_flags);
-
-	    ret = rtdev_open(rtdev);    /* also == 0 if rtdev is already up */
-
-	    if (ret == 0) {
-		mutex_lock(&rtnet_devices_nrt_lock);
-
-		list_for_each(entry, &event_hook_list) {
-		    hook = list_entry(entry, struct rtdev_event_hook, entry);
-		    if (hook->ifup)
-			hook->ifup(rtdev, &cmd);
-		}
-
-		mutex_unlock(&rtnet_devices_nrt_lock);
-	    } else
-		clear_bit(PRIV_FLAG_UP, &rtdev->priv_flags);
-
-	  up_out:
-	    mutex_unlock(&rtdev->nrt_lock);
+	    ret = rtdev_up(rtdev, &cmd);
 	    break;
 
 	case IOC_RT_IFDOWN:
-	    if (mutex_lock_interruptible(&rtdev->nrt_lock))
-		return -ERESTARTSYS;
-
-	    /* spin lock required for sync with routing code */
-	    rtdm_lock_get_irqsave(&rtdev->rtdev_lock, context);
-
-	    if (test_bit(PRIV_FLAG_ADDING_ROUTE, &rtdev->priv_flags)) {
-		rtdm_lock_put_irqrestore(&rtdev->rtdev_lock, context);
-
-		mutex_unlock(&rtdev->nrt_lock);
-		return -EBUSY;
-	    }
-	    clear_bit(PRIV_FLAG_UP, &rtdev->priv_flags);
-
-	    rtdm_lock_put_irqrestore(&rtdev->rtdev_lock, context);
-
-	    ret = 0;
-	    if (rtdev->mac_detach != NULL)
-		ret = rtdev->mac_detach(rtdev);
-
-	    if (ret == 0) {
-		mutex_lock(&rtnet_devices_nrt_lock);
-
-		list_for_each(entry, &event_hook_list) {
-		    hook = list_entry(entry, struct rtdev_event_hook, entry);
-		    if (hook->ifdown)
-			hook->ifdown(rtdev);
-		}
-
-		mutex_unlock(&rtnet_devices_nrt_lock);
-
-		ret = rtdev_close(rtdev);
-	    }
-
-	    mutex_unlock(&rtdev->nrt_lock);
+	    ret = rtdev_down(rtdev);
 	    break;
 
 	case IOC_RT_IFINFO:
-- 
2.17.2

[PATCH 7/9] net/stack: ignore extraneous interface UP/DOWN calls

[Philippe Gerum]

Changing the device flags now implies downing the interface prior to
switching it up anew with a different set of flags.

Return success upon useless calls (e.g. UP when iface is up, DOWN when
iface is down).
---
 kernel/drivers/net/stack/rtdev.c | 39 ++++++++++++++++++--------------
 1 file changed, 22 insertions(+), 17 deletions(-)

diff --git a/kernel/drivers/net/stack/rtdev.c b/kernel/drivers/net/stack/rtdev.c
index 8f3460afd..7f7dcfdd3 100644
--- a/kernel/drivers/net/stack/rtdev.c
+++ b/kernel/drivers/net/stack/rtdev.c
@@ -666,21 +666,24 @@ int rtdev_up(struct rtnet_device *rtdev, struct rtnet_core_cmd *cmd)
 	      IFF_PROMISC) ||
 	     (cmd->args.up.dev_addr_type != ARPHRD_VOID))) {
 		ret = -EBUSY;
-		goto up_out;
+		goto out;
 	}
 
+	if (cmd->args.up.dev_addr_type != ARPHRD_VOID &&
+	    cmd->args.up.dev_addr_type != rtdev->type) {
+		ret = -EINVAL;
+		goto out;
+	}
+
+	/* Skip upon extraneous call only after args have been checked. */
+	if (test_and_set_bit(PRIV_FLAG_UP, &rtdev->priv_flags))
+		goto out;
+
 	rtdev->flags |= cmd->args.up.set_dev_flags;
 	rtdev->flags &= ~cmd->args.up.clear_dev_flags;
 
-	if (cmd->args.up.dev_addr_type != ARPHRD_VOID) {
-		if (cmd->args.up.dev_addr_type != rtdev->type) {
-			ret = -EINVAL;
-			goto up_out;
-		}
+	if (cmd->args.up.dev_addr_type != ARPHRD_VOID)
 		memcpy(rtdev->dev_addr, cmd->args.up.dev_addr, MAX_ADDR_LEN);
-	}
-
-	set_bit(PRIV_FLAG_UP, &rtdev->priv_flags);
 
 	ret = rtdev_open(rtdev);    /* also == 0 if rtdev is already up */
 
@@ -696,8 +699,7 @@ int rtdev_up(struct rtnet_device *rtdev, struct rtnet_core_cmd *cmd)
 		mutex_unlock(&rtnet_devices_nrt_lock);
 	} else
 		clear_bit(PRIV_FLAG_UP, &rtdev->priv_flags);
-
-up_out:
+out:
 	mutex_unlock(&rtdev->nrt_lock);
 
 	return ret;
@@ -718,12 +720,12 @@ int rtdev_down(struct rtnet_device *rtdev)
 	rtdm_lock_get_irqsave(&rtdev->rtdev_lock, context);
 
 	if (test_bit(PRIV_FLAG_ADDING_ROUTE, &rtdev->priv_flags)) {
-		rtdm_lock_put_irqrestore(&rtdev->rtdev_lock, context);
-
-		mutex_unlock(&rtdev->nrt_lock);
-		return -EBUSY;
+		ret = -EBUSY;
+		goto fail;
 	}
-	clear_bit(PRIV_FLAG_UP, &rtdev->priv_flags);
+
+	if (!test_and_clear_bit(PRIV_FLAG_UP, &rtdev->priv_flags))
+		goto fail;
 
 	rtdm_lock_put_irqrestore(&rtdev->rtdev_lock, context);
 
@@ -743,10 +745,13 @@ int rtdev_down(struct rtnet_device *rtdev)
 
 		ret = rtdev_close(rtdev);
 	}
-
+out:
 	mutex_unlock(&rtdev->nrt_lock);
 
 	return ret;
+fail:
+	rtdm_lock_put_irqrestore(&rtdev->rtdev_lock, context);
+	goto out;
 }
 EXPORT_SYMBOL_GPL(rtdev_down);
 
-- 
2.17.2

[PATCH 8/9] net/igb: down interface upon PCI unregister

[Philippe Gerum]

Allow to safely unload the IGB driver module still hosting an active
rtnet interface, by forcing this interface down on behalf on the PCI
device removal handler.
---
 kernel/drivers/net/drivers/igb/igb_main.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/kernel/drivers/net/drivers/igb/igb_main.c b/kernel/drivers/net/drivers/igb/igb_main.c
index 67a34b539..0a6cd4bd6 100644
--- a/kernel/drivers/net/drivers/igb/igb_main.c
+++ b/kernel/drivers/net/drivers/igb/igb_main.c
@@ -2432,6 +2432,7 @@ static void igb_remove(struct pci_dev *pdev)
 	struct igb_adapter *adapter = rtnetdev_priv(netdev);
 	struct e1000_hw *hw = &adapter->hw;
 
+	rtdev_down(netdev);
 	igb_down(adapter);
 
 	pm_runtime_get_noresume(&pdev->dev);
-- 
2.17.2

[PATCH 9/9] net/igb: igb_ioctl() requires execution in secondary mode

[Philippe Gerum]

We may have to call the regular MII layer from this routine.
---
 kernel/drivers/net/drivers/igb/igb_main.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/kernel/drivers/net/drivers/igb/igb_main.c b/kernel/drivers/net/drivers/igb/igb_main.c
index 0a6cd4bd6..c1d1000ea 100644
--- a/kernel/drivers/net/drivers/igb/igb_main.c
+++ b/kernel/drivers/net/drivers/igb/igb_main.c
@@ -5067,6 +5067,9 @@ static int igb_mii_ioctl(struct rtnet_device *netdev, struct ifreq *ifr, int cmd
  **/
 static int igb_ioctl(struct rtnet_device *netdev, struct ifreq *ifr, int cmd)
 {
+	if (rtdm_in_rt_context())
+		return -ENOSYS;
+	
 	switch (cmd) {
 	case SIOCGMIIPHY:
 	case SIOCGMIIREG:
-- 
2.17.2

Re: [PATCH 8/9] net/igb: down interface upon PCI unregister

[Jan Kiszka]

On 26.10.18 16:08, Philippe Gerum wrote:
> Allow to safely unload the IGB driver module still hosting an active
> rtnet interface, by forcing this interface down on behalf on the PCI
> device removal handler.
> ---
>   kernel/drivers/net/drivers/igb/igb_main.c | 1 +
>   1 file changed, 1 insertion(+)
> 
> diff --git a/kernel/drivers/net/drivers/igb/igb_main.c b/kernel/drivers/net/drivers/igb/igb_main.c
> index 67a34b539..0a6cd4bd6 100644
> --- a/kernel/drivers/net/drivers/igb/igb_main.c
> +++ b/kernel/drivers/net/drivers/igb/igb_main.c
> @@ -2432,6 +2432,7 @@ static void igb_remove(struct pci_dev *pdev)
>   	struct igb_adapter *adapter = rtnetdev_priv(netdev);
>   	struct e1000_hw *hw = &adapter->hw;
>   
> +	rtdev_down(netdev);
>   	igb_down(adapter);
>   
>   	pm_runtime_get_noresume(&pdev->dev);
> 

Is that igb-only? Or should be add such call to all driver removals?

Jan

-- 
Siemens AG, Corporate Technology, CT RDA IOT SES-DE
Corporate Competence Center Embedded Linux

Re: [PATCH 8/9] net/igb: down interface upon PCI unregister

[Philippe Gerum]

On 10/31/18 1:32 PM, Jan Kiszka wrote:
> On 26.10.18 16:08, Philippe Gerum wrote:
>> Allow to safely unload the IGB driver module still hosting an active
>> rtnet interface, by forcing this interface down on behalf on the PCI
>> device removal handler.
>> ---
>>   kernel/drivers/net/drivers/igb/igb_main.c | 1 +
>>   1 file changed, 1 insertion(+)
>>
>> diff --git a/kernel/drivers/net/drivers/igb/igb_main.c
>> b/kernel/drivers/net/drivers/igb/igb_main.c
>> index 67a34b539..0a6cd4bd6 100644
>> --- a/kernel/drivers/net/drivers/igb/igb_main.c
>> +++ b/kernel/drivers/net/drivers/igb/igb_main.c
>> @@ -2432,6 +2432,7 @@ static void igb_remove(struct pci_dev *pdev)
>>       struct igb_adapter *adapter = rtnetdev_priv(netdev);
>>       struct e1000_hw *hw = &adapter->hw;
>>   +    rtdev_down(netdev);
>>       igb_down(adapter);
>>         pm_runtime_get_noresume(&pdev->dev);
>>
> 
> Is that igb-only? Or should be add such call to all driver removals?
> 

This is a use case for the new rtnet_down() call applied to IGB which
I'm interested in ATM. If the new up/down API is merged, all drivers
should be updated the same way eventually.

-- 
Philippe.

Re: [PATCH 8/9] net/igb: down interface upon PCI unregister

[Jan Kiszka]

On 31.10.18 15:18, Philippe Gerum wrote:
> On 10/31/18 1:32 PM, Jan Kiszka wrote:
>> On 26.10.18 16:08, Philippe Gerum wrote:
>>> Allow to safely unload the IGB driver module still hosting an active
>>> rtnet interface, by forcing this interface down on behalf on the PCI
>>> device removal handler.
>>> ---
>>>    kernel/drivers/net/drivers/igb/igb_main.c | 1 +
>>>    1 file changed, 1 insertion(+)
>>>
>>> diff --git a/kernel/drivers/net/drivers/igb/igb_main.c
>>> b/kernel/drivers/net/drivers/igb/igb_main.c
>>> index 67a34b539..0a6cd4bd6 100644
>>> --- a/kernel/drivers/net/drivers/igb/igb_main.c
>>> +++ b/kernel/drivers/net/drivers/igb/igb_main.c
>>> @@ -2432,6 +2432,7 @@ static void igb_remove(struct pci_dev *pdev)
>>>        struct igb_adapter *adapter = rtnetdev_priv(netdev);
>>>        struct e1000_hw *hw = &adapter->hw;
>>>    +    rtdev_down(netdev);
>>>        igb_down(adapter);
>>>          pm_runtime_get_noresume(&pdev->dev);
>>>
>>
>> Is that igb-only? Or should be add such call to all driver removals?
>>
> 
> This is a use case for the new rtnet_down() call applied to IGB which
> I'm interested in ATM. If the new up/down API is merged, all drivers
> should be updated the same way eventually.
> 

Then we should probably think about the best pattern now:

Why not do this implicitly when the driver unregisters itself from the RTnet 
core? If that unregistration comes too late with some of them, that would become 
the driver patches.

Jan

-- 
Siemens AG, Corporate Technology, CT RDA IOT SES-DE
Corporate Competence Center Embedded Linux

Re: [PATCH 8/9] net/igb: down interface upon PCI unregister

[Philippe Gerum]

On 10/31/18 4:10 PM, Jan Kiszka wrote:
> On 31.10.18 15:18, Philippe Gerum wrote:
>> On 10/31/18 1:32 PM, Jan Kiszka wrote:
>>> On 26.10.18 16:08, Philippe Gerum wrote:
>>>> Allow to safely unload the IGB driver module still hosting an active
>>>> rtnet interface, by forcing this interface down on behalf on the PCI
>>>> device removal handler.
>>>> ---
>>>>    kernel/drivers/net/drivers/igb/igb_main.c | 1 +
>>>>    1 file changed, 1 insertion(+)
>>>>
>>>> diff --git a/kernel/drivers/net/drivers/igb/igb_main.c
>>>> b/kernel/drivers/net/drivers/igb/igb_main.c
>>>> index 67a34b539..0a6cd4bd6 100644
>>>> --- a/kernel/drivers/net/drivers/igb/igb_main.c
>>>> +++ b/kernel/drivers/net/drivers/igb/igb_main.c
>>>> @@ -2432,6 +2432,7 @@ static void igb_remove(struct pci_dev *pdev)
>>>>        struct igb_adapter *adapter = rtnetdev_priv(netdev);
>>>>        struct e1000_hw *hw = &adapter->hw;
>>>>    +    rtdev_down(netdev);
>>>>        igb_down(adapter);
>>>>          pm_runtime_get_noresume(&pdev->dev);
>>>>
>>>
>>> Is that igb-only? Or should be add such call to all driver removals?
>>>
>>
>> This is a use case for the new rtnet_down() call applied to IGB which
>> I'm interested in ATM. If the new up/down API is merged, all drivers
>> should be updated the same way eventually.
>>
> 
> Then we should probably think about the best pattern now:
> 
> Why not do this implicitly when the driver unregisters itself from the
> RTnet core? If that unregistration comes too late with some of them,
> that would become the driver patches.
> 

Fine by me, however code inspection may not be enough for determining if
such change introduces a regression in some drivers, until actual
testing is done.

Since we don't have all the netdevices at hand and certainly not the
time to test that change against all of them by ourselves, merging the
call into the unregistration service would probably offload this task on
to the users of such adapters in the end.

-- 
Philippe.

Re: [PATCH 8/9] net/igb: down interface upon PCI unregister

[Jan Kiszka]

On 31.10.18 17:32, Philippe Gerum wrote:
> On 10/31/18 4:10 PM, Jan Kiszka wrote:
>> On 31.10.18 15:18, Philippe Gerum wrote:
>>> On 10/31/18 1:32 PM, Jan Kiszka wrote:
>>>> On 26.10.18 16:08, Philippe Gerum wrote:
>>>>> Allow to safely unload the IGB driver module still hosting an active
>>>>> rtnet interface, by forcing this interface down on behalf on the PCI
>>>>> device removal handler.
>>>>> ---
>>>>>     kernel/drivers/net/drivers/igb/igb_main.c | 1 +
>>>>>     1 file changed, 1 insertion(+)
>>>>>
>>>>> diff --git a/kernel/drivers/net/drivers/igb/igb_main.c
>>>>> b/kernel/drivers/net/drivers/igb/igb_main.c
>>>>> index 67a34b539..0a6cd4bd6 100644
>>>>> --- a/kernel/drivers/net/drivers/igb/igb_main.c
>>>>> +++ b/kernel/drivers/net/drivers/igb/igb_main.c
>>>>> @@ -2432,6 +2432,7 @@ static void igb_remove(struct pci_dev *pdev)
>>>>>         struct igb_adapter *adapter = rtnetdev_priv(netdev);
>>>>>         struct e1000_hw *hw = &adapter->hw;
>>>>>     +    rtdev_down(netdev);
>>>>>         igb_down(adapter);
>>>>>           pm_runtime_get_noresume(&pdev->dev);
>>>>>
>>>>
>>>> Is that igb-only? Or should be add such call to all driver removals?
>>>>
>>>
>>> This is a use case for the new rtnet_down() call applied to IGB which
>>> I'm interested in ATM. If the new up/down API is merged, all drivers
>>> should be updated the same way eventually.
>>>
>>
>> Then we should probably think about the best pattern now:
>>
>> Why not do this implicitly when the driver unregisters itself from the
>> RTnet core? If that unregistration comes too late with some of them,
>> that would become the driver patches.
>>
> 
> Fine by me, however code inspection may not be enough for determining if
> such change introduces a regression in some drivers, until actual
> testing is done.
> 
> Since we don't have all the netdevices at hand and certainly not the
> time to test that change against all of them by ourselves, merging the
> call into the unregistration service would probably offload this task on
> to the users of such adapters in the end.
> 

Ok, I'll merge this for now, but before the next on comes around, we should 
really rethink the strategy.

Regarding testing of all adapters: We should probably start ripping out real old 
ones, like all those 100 MBit things from the beginning of this millennium.

Jan

-- 
Siemens AG, Corporate Technology, CT RDA IOT SES-DE
Corporate Competence Center Embedded Linux

Re: [PATCH 4/9] cobalt/ancillaries: prevent false positive with -Wstringop-truncation

[Jan Kiszka]

On 26.10.18 16:08, Philippe Gerum wrote:
> knamecpy() is meant to copying NUL terminated strings even if this
> incurs potential truncation of the source argument if need be, prevent
> GCC 8.x from detecting a false positive when -Wstringop-truncation is
> in effect.
> ---
>   include/cobalt/kernel/ancillaries.h | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/include/cobalt/kernel/ancillaries.h b/include/cobalt/kernel/ancillaries.h
> index cde9856da..b957310c7 100644
> --- a/include/cobalt/kernel/ancillaries.h
> +++ b/include/cobalt/kernel/ancillaries.h
> @@ -58,7 +58,7 @@ void __knamecpy_requires_character_array_as_destination(void);
>   	({								\
>   		if (!__builtin_types_compatible_p(typeof(__dst), char[])) \
>   			__knamecpy_requires_character_array_as_destination();	\
> -		strncpy((__dst), __src, sizeof(__dst) - 1);		\
> +		strncpy((__dst), __src, sizeof(__dst));			\
>   		__dst[sizeof(__dst) - 1] = '\0';			\
>   		__dst;							\
>   	 })
> 

Belatedly picking patch 1-4 of this series into stable to address compilation 
issues against gcc 8.

Jan

-- 
Siemens AG, Corporate Technology, CT RDA IOT SES-DE
Corporate Competence Center Embedded Linux