[PATCH v2 0/4] Enablement of compat tests in CI

[PATCH v2 0/4] Enablement of compat tests in CI

[Florian Bezdeka]

Changes in v2:
  - Removed patch 1 (already merged)
  - Splitted patch 2 into 2 parts (addressing review comments)
    - rtdm extension (now in patch 1)
    - compat support for rt_can (patch 2)
  - Added patch 4 (helps to separate CI logs)

Hi all,

this is a spin-off out of my y2038 queue. This helps to shorten the
y2038 stuff. The following 3 patches are necessary for getting the smokey 
testsuite sucessfully running in compat mode.

The necessary adjustments to the CI infrastructure (part of the
xenomai-images project) will follow soon.

At the end the following issue will be resolved: 
https://gitlab.com/Xenomai/xenomai-hacker-space/-/issues/21

Best regards,
Florian


Florian Bezdeka (4):
  cobalt/rtdm/fd: Adding helpers for handling compat affected data
    structures
  drivers/rtcan: Implement compat support for the rtcan driver
  drivers/net/stack/ipv4/udp: Implement compat support for the udp
    driver
  testsuite/smokey/y2038: Show sizeof(long) and sizeof(time_t) on
    startup

 include/cobalt/kernel/rtdm/fd.h         |  16 ++++
 kernel/cobalt/rtdm/drvlib.c             |  17 +---
 kernel/cobalt/rtdm/fd.c                 | 122 ++++++++++++++++++++++++
 kernel/drivers/can/rtcan_raw.c          |  42 ++++----
 kernel/drivers/net/stack/ipv4/udp/udp.c |  29 +++---
 testsuite/smokey/y2038/syscall-tests.c  |   3 +
 6 files changed, 176 insertions(+), 53 deletions(-)

-- 
2.37.3

[PATCH v2 1/4] cobalt/rtdm/fd: Adding helpers for handling compat affected data structures

[Florian Bezdeka]

Several drivers will have to read and write data structures that have
a different memory layout when handling compat applications. Moving
the decission if the compat path has to be taken into the rtdm layer
helps to keep the drivers simple and clean.

Added helpers
  - for reading and writing struct iovec
    (and its compat representation)
  - for reading and writing struct _rtdm_setsockaddr_args
    (and its compat representation)

Signed-off-by: Florian Bezdeka <florian.bezdeka@siemens.com>
---
 include/cobalt/kernel/rtdm/fd.h |  16 +++++
 kernel/cobalt/rtdm/drvlib.c     |  17 +----
 kernel/cobalt/rtdm/fd.c         | 122 ++++++++++++++++++++++++++++++++
 3 files changed, 140 insertions(+), 15 deletions(-)

diff --git a/include/cobalt/kernel/rtdm/fd.h b/include/cobalt/kernel/rtdm/fd.h
index 058a9c638..176c67e22 100644
--- a/include/cobalt/kernel/rtdm/fd.h
+++ b/include/cobalt/kernel/rtdm/fd.h
@@ -29,6 +29,8 @@
 struct vm_area_struct;
 struct rtdm_fd;
 struct _rtdm_mmap_request;
+struct _rtdm_setsockaddr_args;
+struct _rtdm_setsockopt_args;
 struct xnselector;
 struct cobalt_ppd;
 struct rtdm_device;
@@ -403,6 +405,20 @@ int rtdm_fd_valid_p(int ufd);
 int rtdm_fd_select(int ufd, struct xnselector *selector,
 		   unsigned int type);
 
+int rtdm_fd_get_setsockaddr_args(struct rtdm_fd *fd,
+				 struct _rtdm_setsockaddr_args *dst,
+				 const void *src);
+
+int rtdm_fd_get_setsockopt_args(struct rtdm_fd *fd,
+				struct _rtdm_setsockopt_args *dst,
+				const void *src);
+
+int rtdm_fd_get_iovec(struct rtdm_fd *fd, struct iovec *iov,
+		      const struct user_msghdr *msg, bool rw);
+
+int rtdm_fd_put_iovec(struct rtdm_fd *fd, const struct iovec *iov,
+		      const struct user_msghdr *msg);
+
 int rtdm_device_new_fd(struct rtdm_fd *fd, int ufd,
 		struct rtdm_device *dev);
 
diff --git a/kernel/cobalt/rtdm/drvlib.c b/kernel/cobalt/rtdm/drvlib.c
index 4eaf3a57c..30455c3d7 100644
--- a/kernel/cobalt/rtdm/drvlib.c
+++ b/kernel/cobalt/rtdm/drvlib.c
@@ -2211,14 +2211,7 @@ int rtdm_get_iovec(struct rtdm_fd *fd, struct iovec **iovp,
 		return 0;
 	}
 
-#ifdef CONFIG_XENO_ARCH_SYS3264
-	if (rtdm_fd_is_compat(fd))
-		return sys32_get_iovec(iov,
-			       (struct compat_iovec __user *)msg->msg_iov,
-			       msg->msg_iovlen);
-#endif
-
-	return rtdm_copy_from_user(fd, iov, msg->msg_iov, len);
+	return rtdm_fd_get_iovec(fd, iov, msg, false);
 }
 EXPORT_SYMBOL_GPL(rtdm_get_iovec);
 
@@ -2233,13 +2226,7 @@ int rtdm_put_iovec(struct rtdm_fd *fd, struct iovec *iov,
 		memcpy(msg->msg_iov, iov, len);
 		ret = 0;
 	} else
-#ifdef CONFIG_XENO_ARCH_SYS3264
-		if (rtdm_fd_is_compat(fd))
-			ret = sys32_put_iovec((struct compat_iovec __user *)msg->msg_iov,
-					      iov, msg->msg_iovlen);
-		else
-#endif
-			ret = rtdm_copy_to_user(fd, msg->msg_iov, iov, len);
+		ret = rtdm_fd_put_iovec(fd, iov, msg);
 
 	if (iov != iov_fast)
 		xnfree(iov);
diff --git a/kernel/cobalt/rtdm/fd.c b/kernel/cobalt/rtdm/fd.c
index ed2e13578..29555cb21 100644
--- a/kernel/cobalt/rtdm/fd.c
+++ b/kernel/cobalt/rtdm/fd.c
@@ -31,6 +31,7 @@
 #include <cobalt/kernel/time.h>
 #include <pipeline/inband_work.h>
 #include <trace/events/cobalt-rtdm.h>
+#include <rtdm/compat.h>
 #include <rtdm/fd.h>
 #include "internal.h"
 #include "posix/process.h"
@@ -998,6 +999,127 @@ int rtdm_fd_select(int ufd, struct xnselector *selector,
 	return ret;
 }
 
+int rtdm_fd_get_setsockaddr_args(struct rtdm_fd *fd,
+				 struct _rtdm_setsockaddr_args *dst,
+				 const void *src)
+{
+
+#ifdef CONFIG_XENO_ARCH_SYS3264
+	if (rtdm_fd_is_compat(fd)) {
+		struct compat_rtdm_setsockaddr_args cargs;
+		int ret;
+
+		if (!rtdm_read_user_ok(fd, src, sizeof(cargs)))
+			return -EFAULT;
+
+		ret = rtdm_copy_from_user(fd, &cargs, src, sizeof(cargs));
+		if (ret)
+			return ret;
+
+		dst->addr = compat_ptr(cargs.addr);
+		dst->addrlen = cargs.addrlen;
+
+		return 0;
+	}
+#endif
+
+	if (!rtdm_read_user_ok(fd, src, sizeof(*dst)))
+		return -EFAULT;
+
+	return rtdm_copy_from_user(fd, dst, src, sizeof(*dst));
+}
+EXPORT_SYMBOL_GPL(rtdm_fd_get_setsockaddr_args);
+
+int rtdm_fd_get_setsockopt_args(struct rtdm_fd *fd,
+				struct _rtdm_setsockopt_args *dst,
+				const void *src)
+{
+
+#ifdef CONFIG_XENO_ARCH_SYS3264
+	if (rtdm_fd_is_compat(fd)) {
+		struct compat_rtdm_setsockopt_args cargs;
+		int ret;
+
+		if (!rtdm_read_user_ok(fd, src, sizeof(cargs)))
+			return -EFAULT;
+
+		ret = rtdm_copy_from_user(fd, &cargs, src, sizeof(cargs));
+		if (ret)
+			return ret;
+
+		dst->optlen = cargs.optlen;
+		dst->optval = compat_ptr(cargs.optval);
+		dst->optname = cargs.optname;
+		dst->level = cargs.level;
+
+		return 0;
+	}
+#endif
+
+	if (!rtdm_read_user_ok(fd, src, sizeof(*dst)))
+		return -EFAULT;
+
+	return rtdm_copy_from_user(fd, dst, src, sizeof(*dst));
+}
+EXPORT_SYMBOL_GPL(rtdm_fd_get_setsockopt_args);
+
+int rtdm_fd_get_iovec(struct rtdm_fd *fd, struct iovec *iov,
+		      const struct user_msghdr *msg, bool rw)
+{
+	size_t sz;
+
+#ifdef CONFIG_XENO_ARCH_SYS3264
+	sz = rtdm_fd_is_compat(fd) ? sizeof(struct compat_iovec) : sizeof(*iov);
+#else
+	sz = sizeof(*iov);
+#endif
+
+	sz *= msg->msg_iovlen;
+
+	if (!rw && !rtdm_read_user_ok(fd, msg->msg_iov, sz))
+		return -EFAULT;
+
+	if (rw && !rtdm_rw_user_ok(fd, msg->msg_iov, sz))
+		return -EFAULT;
+
+#ifdef CONFIG_XENO_ARCH_SYS3264
+	if (rtdm_fd_is_compat(fd))
+		return sys32_get_iovec(
+			iov, (struct compat_iovec __user *)msg->msg_iov,
+			(int)msg->msg_iovlen);
+#endif
+
+	return rtdm_copy_from_user(fd, iov, msg->msg_iov, sz);
+}
+EXPORT_SYMBOL_GPL(rtdm_fd_get_iovec);
+
+int rtdm_fd_put_iovec(struct rtdm_fd *fd, const struct iovec *iov,
+		      const struct user_msghdr *msg)
+{
+	size_t sz;
+
+#ifdef CONFIG_XENO_ARCH_SYS3264
+	sz = rtdm_fd_is_compat(fd) ? sizeof(struct compat_iovec) : sizeof(*iov);
+#else
+	sz = sizeof(*iov);
+#endif
+
+	sz *= msg->msg_iovlen;
+
+	if (!rtdm_rw_user_ok(fd, msg->msg_iov, sz))
+		return -EFAULT;
+
+#ifdef CONFIG_XENO_ARCH_SYS3264
+	if (rtdm_fd_is_compat(fd))
+		return sys32_put_iovec(
+			(struct compat_iovec __user *)msg->msg_iov, iov,
+			(int)msg->msg_iovlen);
+#endif
+
+	return rtdm_copy_to_user(fd, msg->msg_iov, iov, sz);
+}
+EXPORT_SYMBOL_GPL(rtdm_fd_put_iovec);
+
 static void destroy_fd(void *cookie, struct xnid *id)
 {
 	struct cobalt_ppd *p = cookie;
-- 
2.37.3

[PATCH v2 2/4] drivers/rtcan: Implement compat support for the rtcan driver

[Florian Bezdeka]

Once Xenomai was running as compat application (=32bit applicaiton over
64bit kernel) the can smokey test failed. This patch fixes the test by
implementing the necessary compat bits.

Affected ioctls:
  - _RTIOC_BIND
  - _RTIOC_SETSOCKOPT

Affected syscalls:
  - recvmsg
  - sendmsg

Signed-off-by: Florian Bezdeka <florian.bezdeka@siemens.com>
---
 kernel/drivers/can/rtcan_raw.c | 42 ++++++++++++++--------------------
 1 file changed, 17 insertions(+), 25 deletions(-)

diff --git a/kernel/drivers/can/rtcan_raw.c b/kernel/drivers/can/rtcan_raw.c
index dffc6e837..92b7617fb 100644
--- a/kernel/drivers/can/rtcan_raw.c
+++ b/kernel/drivers/can/rtcan_raw.c
@@ -41,6 +41,7 @@
 #include <rtdm/driver.h>
 
 #include <rtdm/can.h>
+#include <rtdm/compat.h>
 #include "rtcan_version.h"
 #include "rtcan_socket.h"
 #include "rtcan_list.h"
@@ -411,17 +412,15 @@ int rtcan_raw_ioctl(struct rtdm_fd *fd,
     int ret = 0;
 
     switch (request) {
-    case _RTIOC_BIND: {
+    COMPAT_CASE(_RTIOC_BIND): {
+
 	struct _rtdm_setsockaddr_args *setaddr, setaddr_buf;
 	struct sockaddr_can *sockaddr, sockaddr_buf;
 
 	if (rtdm_fd_is_user(fd)) {
-	    /* Copy argument structure from userspace */
-	    if (!rtdm_read_user_ok(fd, arg,
-				   sizeof(struct _rtdm_setsockaddr_args)) ||
-		rtdm_copy_from_user(fd, &setaddr_buf, arg,
-				    sizeof(struct _rtdm_setsockaddr_args)))
-		return -EFAULT;
+	    ret = rtdm_fd_get_setsockaddr_args(fd, &setaddr_buf, arg);
+	    if (ret)
+		return ret;
 
 	    setaddr = &setaddr_buf;
 
@@ -447,16 +446,14 @@ int rtcan_raw_ioctl(struct rtdm_fd *fd,
 	break;
     }
 
-    case _RTIOC_SETSOCKOPT: {
+    COMPAT_CASE(_RTIOC_SETSOCKOPT): {
 	struct _rtdm_setsockopt_args *setopt;
 	struct _rtdm_setsockopt_args setopt_buf;
 
 	if (rtdm_fd_is_user(fd)) {
-	    if (!rtdm_read_user_ok(fd, arg,
-				   sizeof(struct _rtdm_setsockopt_args)) ||
-		rtdm_copy_from_user(fd, &setopt_buf, arg,
-				    sizeof(struct _rtdm_setsockopt_args)))
-		return -EFAULT;
+	    ret = rtdm_fd_get_setsockopt_args(fd, &setopt_buf, arg);
+	    if (ret)
+		    return ret;
 
 	    setopt = &setopt_buf;
 	} else
@@ -575,11 +572,9 @@ ssize_t rtcan_raw_recvmsg(struct rtdm_fd *fd,
 
     if (rtdm_fd_is_user(fd)) {
 	/* Copy IO vector from userspace */
-	if (!rtdm_rw_user_ok(fd, msg->msg_iov,
-			     sizeof(struct iovec)) ||
-	    rtdm_copy_from_user(fd, &iov_buf, msg->msg_iov,
-				sizeof(struct iovec)))
-	    return -EFAULT;
+	ret = rtdm_fd_get_iovec(fd, &iov_buf, msg, true);
+	if (ret)
+		return -EFAULT;
 
 	iov = &iov_buf;
     }
@@ -716,9 +711,9 @@ ssize_t rtcan_raw_recvmsg(struct rtdm_fd *fd,
 	iov->iov_base += sizeof(can_frame_t);
 	iov->iov_len -= sizeof(can_frame_t);
 	/* ... and copy it, too. */
-	if (rtdm_copy_to_user(fd, msg->msg_iov, iov,
-			      sizeof(struct iovec)))
-	    return -EFAULT;
+	ret = rtdm_fd_put_iovec(fd, iov, msg);
+	if (ret)
+		return -EFAULT;
 
 	/* Copy timestamp if existent and wanted */
 	if (msg->msg_controllen) {
@@ -922,10 +917,7 @@ ssize_t rtcan_raw_sendmsg(struct rtdm_fd *fd,
     if (rtdm_fd_is_user(fd)) {
 
 	/* Copy IO vector from userspace */
-	if (!rtdm_rw_user_ok(fd, msg->msg_iov,
-			     sizeof(struct iovec)) ||
-	    rtdm_copy_from_user(fd, &iov_buf, msg->msg_iov,
-				sizeof(struct iovec))) {
+	if (rtdm_fd_get_iovec(fd, &iov_buf, msg, false)) {
 	    ret = -EFAULT;
 	    goto finally;
 	}
-- 
2.37.3

[PATCH v2 3/4] drivers/net/stack/ipv4/udp: Implement compat support for the udp driver

[Florian Bezdeka]

The rtnet interface was lacking compat support for the following ioctl
commands:
  - _RTIOC_BIND
  - _RTIOC_CONNECT

This patch is heavily inspired by Konstantin's proposal but migrated
to some rtnet APIs that allow us to stay away from
  #ifdef CONFIG_XENO_ARCH_SYS3264
sections.

Reported-by: Konstantin Smola <ksmola51@gmail.com>
Signed-off-by: Florian Bezdeka <florian.bezdeka@siemens.com>
---
 kernel/drivers/net/stack/ipv4/udp/udp.c | 29 ++++++++++++++-----------
 1 file changed, 16 insertions(+), 13 deletions(-)

diff --git a/kernel/drivers/net/stack/ipv4/udp/udp.c b/kernel/drivers/net/stack/ipv4/udp/udp.c
index 5977a3bc2..777c8a27a 100644
--- a/kernel/drivers/net/stack/ipv4/udp/udp.c
+++ b/kernel/drivers/net/stack/ipv4/udp/udp.c
@@ -31,6 +31,7 @@
 #include <linux/tcp.h>
 #include <linux/list.h>
 
+#include <rtdm/compat.h>
 #include <rtskb.h>
 #include <rtnet_internal.h>
 #include <rtnet_checksum.h>
@@ -355,25 +356,27 @@ void rt_udp_close(struct rtdm_fd *fd)
 int rt_udp_ioctl(struct rtdm_fd *fd, unsigned int request, void __user *arg)
 {
 	struct rtsocket *sock = rtdm_fd_to_private(fd);
-	const struct _rtdm_setsockaddr_args *setaddr;
-	struct _rtdm_setsockaddr_args _setaddr;
+	struct _rtdm_setsockaddr_args args;
+	bool do_bind = false;
+	int ret;
 
 	/* fast path for common socket IOCTLs */
 	if (_IOC_TYPE(request) == RTIOC_TYPE_NETWORK)
 		return rt_socket_common_ioctl(fd, request, arg);
 
 	switch (request) {
-	case _RTIOC_BIND:
-	case _RTIOC_CONNECT:
-		setaddr = rtnet_get_arg(fd, &_setaddr, arg, sizeof(_setaddr));
-		if (IS_ERR(setaddr))
-			return PTR_ERR(setaddr);
-		if (request == _RTIOC_BIND)
-			return rt_udp_bind(fd, sock, setaddr->addr,
-					   setaddr->addrlen);
-
-		return rt_udp_connect(fd, sock, setaddr->addr,
-				      setaddr->addrlen);
+	COMPAT_CASE(_RTIOC_BIND):
+		do_bind = true;
+		fallthrough;
+	COMPAT_CASE(_RTIOC_CONNECT):
+		ret = rtdm_fd_get_setsockaddr_args(fd, &args, arg);
+		if (ret)
+			return ret;
+
+		if (do_bind)
+			return rt_udp_bind(fd, sock, args.addr, args.addrlen);
+
+		return rt_udp_connect(fd, sock, args.addr, args.addrlen);
 
 	default:
 		return rt_ip_ioctl(fd, request, arg);
-- 
2.37.3

[PATCH v2 4/4] testsuite/smokey/y2038: Show sizeof(long) and sizeof(time_t) on startup

[Florian Bezdeka]

This helps to identify if the testsuite was run in compat mode and if
y2038 support has been requested. Useful when looking at CI logs and
trying to understand the runtime conditions that caused a failure.

Signed-off-by: Florian Bezdeka <florian.bezdeka@siemens.com>
---
 testsuite/smokey/y2038/syscall-tests.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/testsuite/smokey/y2038/syscall-tests.c b/testsuite/smokey/y2038/syscall-tests.c
index 2f4041e51..b1abec260 100644
--- a/testsuite/smokey/y2038/syscall-tests.c
+++ b/testsuite/smokey/y2038/syscall-tests.c
@@ -1237,6 +1237,9 @@ static int run_y2038(struct smokey_test *t, int argc, char *const argv[])
 {
 	int ret;
 
+	smokey_note("y2038: sizeof(long) = %zu, sizeof(time_t) = %zu",
+		    sizeof(long), sizeof(time_t));
+
 	ret = check_kernel_version();
 	if (ret)
 		return (ret < 0) ? ret : 0; /* skip if no y2038 safe kernel */
-- 
2.37.3

Re: [PATCH v2 0/4] Enablement of compat tests in CI

[Jan Kiszka]

On 20.09.22 14:24, Florian Bezdeka wrote:
> Changes in v2:
>   - Removed patch 1 (already merged)
>   - Splitted patch 2 into 2 parts (addressing review comments)
>     - rtdm extension (now in patch 1)
>     - compat support for rt_can (patch 2)
>   - Added patch 4 (helps to separate CI logs)
> 

How about using patch 1 on other drivers than thouse you address in
patch 2 and 3? Not needed? Or not possible?

Jan

> Hi all,
> 
> this is a spin-off out of my y2038 queue. This helps to shorten the
> y2038 stuff. The following 3 patches are necessary for getting the smokey 
> testsuite sucessfully running in compat mode.
> 
> The necessary adjustments to the CI infrastructure (part of the
> xenomai-images project) will follow soon.
> 
> At the end the following issue will be resolved: 
> https://gitlab.com/Xenomai/xenomai-hacker-space/-/issues/21
> 
> Best regards,
> Florian
> 
> 
> Florian Bezdeka (4):
>   cobalt/rtdm/fd: Adding helpers for handling compat affected data
>     structures
>   drivers/rtcan: Implement compat support for the rtcan driver
>   drivers/net/stack/ipv4/udp: Implement compat support for the udp
>     driver
>   testsuite/smokey/y2038: Show sizeof(long) and sizeof(time_t) on
>     startup
> 
>  include/cobalt/kernel/rtdm/fd.h         |  16 ++++
>  kernel/cobalt/rtdm/drvlib.c             |  17 +---
>  kernel/cobalt/rtdm/fd.c                 | 122 ++++++++++++++++++++++++
>  kernel/drivers/can/rtcan_raw.c          |  42 ++++----
>  kernel/drivers/net/stack/ipv4/udp/udp.c |  29 +++---
>  testsuite/smokey/y2038/syscall-tests.c  |   3 +
>  6 files changed, 176 insertions(+), 53 deletions(-)
> 

-- 
Siemens AG, Technology
Competence Center Embedded Linux

Re: [PATCH v2 0/4] Enablement of compat tests in CI

[Bezdeka, Florian]

On Mon, 2022-09-26 at 16:12 +0200, Jan Kiszka wrote:
> On 20.09.22 14:24, Florian Bezdeka wrote:
> > Changes in v2:
> >   - Removed patch 1 (already merged)
> >   - Splitted patch 2 into 2 parts (addressing review comments)
> >     - rtdm extension (now in patch 1)
> >     - compat support for rt_can (patch 2)
> >   - Added patch 4 (helps to separate CI logs)
> > 
> 
> How about using patch 1 on other drivers than thouse you address in
> patch 2 and 3? Not needed? Or not possible?

No (failing) tests available. That's the main reason. There might be
more that is missing regarding compat support in those drivers. As this
is more compat than y2038 related I didn't spent to much time on that
for now.

If you like we could add potential compat problematic drivers to some
kind of todo / issue list.

Florian

> 
> Jan
> 
> > Hi all,
> > 
> > this is a spin-off out of my y2038 queue. This helps to shorten the
> > y2038 stuff. The following 3 patches are necessary for getting the smokey 
> > testsuite sucessfully running in compat mode.
> > 
> > The necessary adjustments to the CI infrastructure (part of the
> > xenomai-images project) will follow soon.
> > 
> > At the end the following issue will be resolved: 
> > https://gitlab.com/Xenomai/xenomai-hacker-space/-/issues/21
> > 
> > Best regards,
> > Florian
> > 
> > 
> > Florian Bezdeka (4):
> >   cobalt/rtdm/fd: Adding helpers for handling compat affected data
> >     structures
> >   drivers/rtcan: Implement compat support for the rtcan driver
> >   drivers/net/stack/ipv4/udp: Implement compat support for the udp
> >     driver
> >   testsuite/smokey/y2038: Show sizeof(long) and sizeof(time_t) on
> >     startup
> > 
> >  include/cobalt/kernel/rtdm/fd.h         |  16 ++++
> >  kernel/cobalt/rtdm/drvlib.c             |  17 +---
> >  kernel/cobalt/rtdm/fd.c                 | 122 ++++++++++++++++++++++++
> >  kernel/drivers/can/rtcan_raw.c          |  42 ++++----
> >  kernel/drivers/net/stack/ipv4/udp/udp.c |  29 +++---
> >  testsuite/smokey/y2038/syscall-tests.c  |   3 +
> >  6 files changed, 176 insertions(+), 53 deletions(-)
> > 
> 

Re: [PATCH v2 0/4] Enablement of compat tests in CI

[Jan Kiszka]

On 26.09.22 16:21, Bezdeka, Florian (T CED SES-DE) wrote:
> On Mon, 2022-09-26 at 16:12 +0200, Jan Kiszka wrote:
>> On 20.09.22 14:24, Florian Bezdeka wrote:
>>> Changes in v2:
>>>   - Removed patch 1 (already merged)
>>>   - Splitted patch 2 into 2 parts (addressing review comments)
>>>     - rtdm extension (now in patch 1)
>>>     - compat support for rt_can (patch 2)
>>>   - Added patch 4 (helps to separate CI logs)
>>>
>>
>> How about using patch 1 on other drivers than thouse you address in
>> patch 2 and 3? Not needed? Or not possible?
> 
> No (failing) tests available. That's the main reason. There might be
> more that is missing regarding compat support in those drivers. As this
> is more compat than y2038 related I didn't spent to much time on that
> for now.
> 
> If you like we could add potential compat problematic drivers to some
> kind of todo / issue list.

I'm picking on this to ensure that the new RTDM APIs have at least been
validated against all potential in-tree users. So, could you have a look if

kernel/drivers/ipc/*
kernel/drivers/net/stack/iovec.c
kernel/drivers/net/stack/ipv4/tcp/tcp.c
kernel/drivers/net/stack/packet/af_packet.c

may have a need AND could make use of the functions? Actually writing
patches could still be done later.

Thanks,
Jan

-- 
Siemens AG, Technology
Competence Center Embedded Linux

Re: [PATCH v2 0/4] Enablement of compat tests in CI

[Jan Kiszka]

On 26.09.22 16:57, Jan Kiszka wrote:
> On 26.09.22 16:21, Bezdeka, Florian (T CED SES-DE) wrote:
>> On Mon, 2022-09-26 at 16:12 +0200, Jan Kiszka wrote:
>>> On 20.09.22 14:24, Florian Bezdeka wrote:
>>>> Changes in v2:
>>>>   - Removed patch 1 (already merged)
>>>>   - Splitted patch 2 into 2 parts (addressing review comments)
>>>>     - rtdm extension (now in patch 1)
>>>>     - compat support for rt_can (patch 2)
>>>>   - Added patch 4 (helps to separate CI logs)
>>>>
>>>
>>> How about using patch 1 on other drivers than thouse you address in
>>> patch 2 and 3? Not needed? Or not possible?
>>
>> No (failing) tests available. That's the main reason. There might be
>> more that is missing regarding compat support in those drivers. As this
>> is more compat than y2038 related I didn't spent to much time on that
>> for now.
>>
>> If you like we could add potential compat problematic drivers to some
>> kind of todo / issue list.
> 
> I'm picking on this to ensure that the new RTDM APIs have at least been
> validated against all potential in-tree users. So, could you have a look if
> 
> kernel/drivers/ipc/*
> kernel/drivers/net/stack/iovec.c
> kernel/drivers/net/stack/ipv4/tcp/tcp.c
> kernel/drivers/net/stack/packet/af_packet.c
> 
> may have a need AND could make use of the functions? Actually writing
> patches could still be done later.
> 
> Thanks,
> Jan
> 

Thanks said, I'm merging the series into next now as it fixes issues and
allows to progress. But we should validate the API before things move on
into master.

Jan

-- 
Siemens AG, Technology
Competence Center Embedded Linux

Re: [PATCH v2 0/4] Enablement of compat tests in CI

[Bezdeka, Florian]

On Mon, 2022-09-26 at 16:58 +0200, Jan Kiszka wrote:
> On 26.09.22 16:57, Jan Kiszka wrote:
> > On 26.09.22 16:21, Bezdeka, Florian (T CED SES-DE) wrote:
> > > On Mon, 2022-09-26 at 16:12 +0200, Jan Kiszka wrote:
> > > > On 20.09.22 14:24, Florian Bezdeka wrote:
> > > > > Changes in v2:
> > > > >   - Removed patch 1 (already merged)
> > > > >   - Splitted patch 2 into 2 parts (addressing review comments)
> > > > >     - rtdm extension (now in patch 1)
> > > > >     - compat support for rt_can (patch 2)
> > > > >   - Added patch 4 (helps to separate CI logs)
> > > > > 
> > > > 
> > > > How about using patch 1 on other drivers than thouse you address in
> > > > patch 2 and 3? Not needed? Or not possible?
> > > 
> > > No (failing) tests available. That's the main reason. There might be
> > > more that is missing regarding compat support in those drivers. As this
> > > is more compat than y2038 related I didn't spent to much time on that
> > > for now.
> > > 
> > > If you like we could add potential compat problematic drivers to some
> > > kind of todo / issue list.
> > 
> > I'm picking on this to ensure that the new RTDM APIs have at least been
> > validated against all potential in-tree users. So, could you have a look if
> > 
> > kernel/drivers/ipc/*

bufp.c, iddp.c, xddp.c: Missing part was around struct iovec which is
now handled correctly.

> > kernel/drivers/net/stack/iovec.c

iovec.c looks OK. It is working on struct iovec that already has been
correctly filled up by rtdm_{get,put}_iovec() upfront. Copying data
from struct iovec::iov_base is not different in compat mode.

> > kernel/drivers/net/stack/ipv4/tcp/tcp.c
> > kernel/drivers/net/stack/packet/af_packet.c

regarding iovec:
rtdm_{get,put}_iovec is used - which now handles compat mode correctly
- so no additional API adjustments necessary.

regarding ioctl:
The existing API is usable as is but some additional helpers might be
necessary for handling struct _rtdm_getsockaddr_args and struct
_rtdm_getsockopt_args.

Summary: I assume the API can stay as is.

> > 
> > may have a need AND could make use of the functions? Actually writing
> > patches could still be done later.
> > 
> > Thanks,
> > Jan
> > 
> 
> Thanks said, I'm merging the series into next now as it fixes issues and
> allows to progress. But we should validate the API before things move on
> into master.
> 
> Jan
> 

Re: [PATCH v2 2/4] drivers/rtcan: Implement compat support for the rtcan driver

[Bezdeka, Florian]

On Tue, 2022-09-20 at 14:25 +0200, Florian Bezdeka wrote:
> Once Xenomai was running as compat application (=32bit applicaiton over
> 64bit kernel) the can smokey test failed. This patch fixes the test by
> implementing the necessary compat bits.
> 
> Affected ioctls:
>   - _RTIOC_BIND
>   - _RTIOC_SETSOCKOPT
> 
> Affected syscalls:
>   - recvmsg
>   - sendmsg
> 
> Signed-off-by: Florian Bezdeka <florian.bezdeka@siemens.com>
> ---
>  kernel/drivers/can/rtcan_raw.c | 42 ++++++++++++++--------------------
>  1 file changed, 17 insertions(+), 25 deletions(-)
> 

<snip>


> @@ -922,10 +917,7 @@ ssize_t rtcan_raw_sendmsg(struct rtdm_fd *fd,
>      if (rtdm_fd_is_user(fd)) {
>  
>  	/* Copy IO vector from userspace */
> -	if (!rtdm_rw_user_ok(fd, msg->msg_iov,
> -			     sizeof(struct iovec)) ||
> -	    rtdm_copy_from_user(fd, &iov_buf, msg->msg_iov,
> -				sizeof(struct iovec))) {
> +	if (rtdm_fd_get_iovec(fd, &iov_buf, msg, false)) {
                                                 ^^
To get the rw check the last parameter should have been true here.
Noticed while going through all that again to make sure the API is
correct.

Jan, this is already part of next, do you want me to send v3 or could
you directly fix that up?

Best regards,
Florian

>  	    ret = -EFAULT;
>  	    goto finally;
>  	}

Re: [PATCH v2 2/4] drivers/rtcan: Implement compat support for the rtcan driver

[Jan Kiszka]

On 01.10.22 00:48, Bezdeka, Florian (T CED SES-DE) wrote:
> On Tue, 2022-09-20 at 14:25 +0200, Florian Bezdeka wrote:
>> Once Xenomai was running as compat application (=32bit applicaiton over
>> 64bit kernel) the can smokey test failed. This patch fixes the test by
>> implementing the necessary compat bits.
>>
>> Affected ioctls:
>>   - _RTIOC_BIND
>>   - _RTIOC_SETSOCKOPT
>>
>> Affected syscalls:
>>   - recvmsg
>>   - sendmsg
>>
>> Signed-off-by: Florian Bezdeka <florian.bezdeka@siemens.com>
>> ---
>>  kernel/drivers/can/rtcan_raw.c | 42 ++++++++++++++--------------------
>>  1 file changed, 17 insertions(+), 25 deletions(-)
>>
> 
> <snip>
> 
> 
>> @@ -922,10 +917,7 @@ ssize_t rtcan_raw_sendmsg(struct rtdm_fd *fd,
>>      if (rtdm_fd_is_user(fd)) {
>>
>>       /* Copy IO vector from userspace */
>> -     if (!rtdm_rw_user_ok(fd, msg->msg_iov,
>> -                          sizeof(struct iovec)) ||
>> -         rtdm_copy_from_user(fd, &iov_buf, msg->msg_iov,
>> -                             sizeof(struct iovec))) {
>> +     if (rtdm_fd_get_iovec(fd, &iov_buf, msg, false)) {
>                                                  ^^
> To get the rw check the last parameter should have been true here.
> Noticed while going through all that again to make sure the API is
> correct.
> 
> Jan, this is already part of next, do you want me to send v3 or could
> you directly fix that up?
> 

Is that control relevant at all? Aren't we always writing something back?

Jan

-- 
Siemens AG, Technology
Competence Center Embedded Linux

Re: [PATCH v2 2/4] drivers/rtcan: Implement compat support for the rtcan driver

[Bezdeka, Florian]

On Tue, 2022-10-04 at 09:46 +0200, Jan Kiszka wrote:
> On 01.10.22 00:48, Bezdeka, Florian (T CED SES-DE) wrote:
> > On Tue, 2022-09-20 at 14:25 +0200, Florian Bezdeka wrote:
> > > Once Xenomai was running as compat application (=32bit applicaiton over
> > > 64bit kernel) the can smokey test failed. This patch fixes the test by
> > > implementing the necessary compat bits.
> > > 
> > > Affected ioctls:
> > >   - _RTIOC_BIND
> > >   - _RTIOC_SETSOCKOPT
> > > 
> > > Affected syscalls:
> > >   - recvmsg
> > >   - sendmsg
> > > 
> > > Signed-off-by: Florian Bezdeka <florian.bezdeka@siemens.com>
> > > ---
> > >  kernel/drivers/can/rtcan_raw.c | 42 ++++++++++++++--------------------
> > >  1 file changed, 17 insertions(+), 25 deletions(-)
> > > 
> > 
> > <snip>
> > 
> > 
> > > @@ -922,10 +917,7 @@ ssize_t rtcan_raw_sendmsg(struct rtdm_fd *fd,
> > >      if (rtdm_fd_is_user(fd)) {
> > > 
> > >       /* Copy IO vector from userspace */
> > > -     if (!rtdm_rw_user_ok(fd, msg->msg_iov,
> > > -                          sizeof(struct iovec)) ||
> > > -         rtdm_copy_from_user(fd, &iov_buf, msg->msg_iov,
> > > -                             sizeof(struct iovec))) {
> > > +     if (rtdm_fd_get_iovec(fd, &iov_buf, msg, false)) {
> >                                                  ^^
> > To get the rw check the last parameter should have been true here.
> > Noticed while going through all that again to make sure the API is
> > correct.
> > 
> > Jan, this is already part of next, do you want me to send v3 or could
> > you directly fix that up?
> > 
> 
> Is that control relevant at all? Aren't we always writing something back?

It has been a while, but I was able to re-visit this one today. It's
fine. We are in the TX path here, so we're reading only. The RX path
will write back so a rw check is necessary. I fixed a bug without
noticing it.

Florian
> 
> Jan
>