Here's an updated patch addressing mentioned issues. I've noticed ipc/internal.h re-defines COMPAT_CASE(__op) as well.
I don't know what gmail is doing to the inline text (this should be plain text) so I've also attached a .gz version of the patch. Subject: [PATCH] 32-bit userspace app support for rtcan Signed-off-by: C Smith <[email protected]> --- kernel/drivers/can/rtcan_internal.h | 1 + kernel/drivers/can/rtcan_raw.c | 102 +++++++++++++++++++++++------------- 2 files changed, 67 insertions(+), 36 deletions(-) diff --git a/kernel/drivers/can/rtcan_internal.h b/kernel/drivers/can/rtcan_internal.h index b290005..d1f9d31 100644 --- a/kernel/drivers/can/rtcan_internal.h +++ b/kernel/drivers/can/rtcan_internal.h @@ -28,6 +28,7 @@ #include <linux/module.h> #include <rtdm/driver.h> +#include <rtdm/compat.h> #ifdef CONFIG_XENO_DRIVERS_CAN_DEBUG #define RTCAN_ASSERT(expr, func) \ diff --git a/kernel/drivers/can/rtcan_raw.c b/kernel/drivers/can/rtcan_raw.c index 693b927..88a179f 100644 --- a/kernel/drivers/can/rtcan_raw.c +++ b/kernel/drivers/can/rtcan_raw.c @@ -215,6 +215,53 @@ EXPORT_SYMBOL_GPL(rtcan_loopback); #endif /* CONFIG_XENO_DRIVERS_CAN_LOOPBACK */ +int rtcan_get_sockaddr(struct rtdm_fd *fd, struct sockaddr_can **saddrp, + const void *arg) +{ + struct _rtdm_setsockaddr_args *setaddr, setaddr_buf; + int ret = 0; + + if (!rtdm_fd_is_user(fd)) { + setaddr = (struct _rtdm_setsockaddr_args *)arg; + *saddrp = (struct sockaddr_can *)setaddr->addr; + } + +#ifdef CONFIG_XENO_ARCH_SYS3264 + if (rtdm_fd_is_compat(fd)) { + struct compat_rtdm_setsockaddr_args csetaddr_buff; + + /* Copy argument structure from userspace */ + ret = rtdm_safe_copy_from_user(fd, &csetaddr_buff, + arg, sizeof(csetaddr_buff)); + if (ret) + return ret; + + /* Check size */ + if (csetaddr_buff.addrlen != sizeof(struct sockaddr_can)) + return -EINVAL; + + return rtdm_safe_copy_from_user(fd, *saddrp, + compat_ptr(csetaddr_buff.addr), + sizeof(struct sockaddr_can)); + } +#endif + + if (rtdm_safe_copy_from_user(fd, &setaddr_buf, arg, + sizeof(struct _rtdm_setsockaddr_args))) + return -EFAULT; + + setaddr = &setaddr_buf; + + /* Check size */ + if (setaddr->addrlen != sizeof(struct sockaddr_can)) + return -EINVAL; + + return rtdm_safe_copy_from_user(fd, *saddrp, + setaddr->addr, + sizeof(struct sockaddr_can)); +} + + int rtcan_raw_socket(struct rtdm_fd *fd, int protocol) { /* Only protocol CAN_RAW is supported */ @@ -408,46 +455,21 @@ static int rtcan_raw_setsockopt(struct rtdm_fd *fd, int rtcan_raw_ioctl(struct rtdm_fd *fd, unsigned int request, void *arg) { + struct sockaddr_can saddr, *saddrp = &saddr; int ret = 0; switch (request) { - 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; - - setaddr = &setaddr_buf; - - /* Check size */ - if (setaddr->addrlen != sizeof(struct sockaddr_can)) - return -EINVAL; - - /* Copy argument structure from userspace */ - if (!rtdm_read_user_ok(fd, arg, - sizeof(struct sockaddr_can)) || - rtdm_copy_from_user(fd, &sockaddr_buf, setaddr->addr, - sizeof(struct sockaddr_can))) - return -EFAULT; - sockaddr = &sockaddr_buf; - } else { - setaddr = (struct _rtdm_setsockaddr_args *)arg; - sockaddr = (struct sockaddr_can *)setaddr->addr; - } - - /* Now, all required data are in kernel space */ - ret = rtcan_raw_bind(fd, sockaddr); + COMPAT_CASE(_RTIOC_BIND): + ret = rtcan_get_sockaddr(fd, &saddrp, arg); + if (ret) + return ret; + if (saddrp == NULL) + return -EFAULT; + ret = rtcan_raw_bind(fd, saddrp); break; - } - case _RTIOC_SETSOCKOPT: { + COMPAT_CASE(_RTIOC_SETSOCKOPT): { struct _rtdm_setsockopt_args *setopt; struct _rtdm_setsockopt_args setopt_buf; @@ -532,7 +554,7 @@ ssize_t rtcan_raw_recvmsg(struct rtdm_fd *fd, struct rtcan_socket *sock = rtdm_fd_to_private(fd); struct sockaddr_can scan; nanosecs_rel_t timeout; - struct iovec *iov = (struct iovec *)msg->msg_iov; + struct iovec iov_fast[RTDM_IOV_FASTMAX], *iov; struct iovec iov_buf; can_frame_t frame; nanosecs_abs_t timestamp = 0; @@ -584,6 +606,10 @@ ssize_t rtcan_raw_recvmsg(struct rtdm_fd *fd, iov = &iov_buf; } + ret = rtdm_get_iovec(fd, &iov, msg, iov_fast); + if (ret) + return ret; + /* Check size of buffer */ if (iov->iov_len < sizeof(can_frame_t)) return -EMSGSIZE; @@ -768,7 +794,7 @@ ssize_t rtcan_raw_sendmsg(struct rtdm_fd *fd, struct rtcan_socket *sock = rtdm_fd_to_private(fd); struct sockaddr_can *scan = (struct sockaddr_can *)msg->msg_name; struct sockaddr_can scan_buf; - struct iovec *iov = (struct iovec *)msg->msg_iov; + struct iovec iov_fast[RTDM_IOV_FASTMAX], *iov; struct iovec iov_buf; can_frame_t *frame; can_frame_t frame_buf; @@ -841,6 +867,10 @@ ssize_t rtcan_raw_sendmsg(struct rtdm_fd *fd, iov = &iov_buf; } + ret = rtdm_get_iovec(fd, &iov, msg, iov_fast); + if (ret) + return ret; + /* Check size of buffer */ if (iov->iov_len != sizeof(can_frame_t)) return -EMSGSIZE; -- 2.1.0 On Fri, Nov 5, 2021 at 1:25 AM Bezdeka, Florian <[email protected]> wrote: > > On Fri, 2021-11-05 at 09:14 +0100, Jan Kiszka wrote: > > On 05.11.21 08:09, C Smith wrote: > > > Please review and accept this patch, which allows 32 bit apps to send > > > and receive CAN. It is tested successfully with 32bit and 64bit compiles > > > of utils/can/rtcansend.c / and rtcanrecv.c. The compat interface is now > > > implemented, rtdm_get_iovec() is used and the get_sockaddr() methodology > > > from the xddp sockets code was emulated to get the CAN receive to work. > > > > Please write a commit message in a way that it motivates the change. Any > > introductory words can go into a cover letter or... > > > > > > > > Signed-off-by: C Smith <[email protected] > > > <mailto:[email protected]>> > > > --- > > > > ...after this separator. > > > > > kernel/drivers/can/rtcan_internal.h | 3 + > > > kernel/drivers/can/rtcan_raw.c | 111 > > > ++++++++++++++++++++++++------------ > > > 2 files changed, 76 insertions(+), 38 deletions(-) > > > > > > > Not sure what the baseline was or if something was mangled, but the > > patch does not apply on top of master/next. > > > > > diff --git a/kernel/drivers/can/rtcan_internal.h > > > b/kernel/drivers/can/rtcan_internal.h > > > index b290005..03febef 100644 > > > --- a/kernel/drivers/can/rtcan_internal.h > > > +++ b/kernel/drivers/can/rtcan_internal.h > > > @@ -28,6 +28,7 @@ > > > > > > #include <linux/module.h> > > > #include <rtdm/driver.h> > > > +#include <rtdm/compat.h> > > > > > > #ifdef CONFIG_XENO_DRIVERS_CAN_DEBUG > > > #define RTCAN_ASSERT(expr, func) \ > > > @@ -58,4 +59,6 @@ > > > #define rtcandev_err(dev, fmt, args...) \ > > > printk(KERN_ERR "%s: " fmt, (dev)->name, ##args) > > > > > > +#define COMPAT_CASE(__op) case __op __COMPAT_CASE(__op ## _COMPAT) > > > + > > > > Why redefing this? It's part of rtdm/compat.h already, isn't it? > > > > > #endif /* __RTCAN_INTERNAL_H_ */ > > > diff --git a/kernel/drivers/can/rtcan_raw.c > > > b/kernel/drivers/can/rtcan_raw.c > > > index 693b927..6e8ebe6 100644 > > > --- a/kernel/drivers/can/rtcan_raw.c > > > +++ b/kernel/drivers/can/rtcan_raw.c > > > @@ -215,6 +215,58 @@ EXPORT_SYMBOL_GPL(rtcan_loopback); > > > #endif /* CONFIG_XENO_DRIVERS_CAN_LOOPBACK */ > > > > > > > > > +int rtcan_get_sockaddr(struct rtdm_fd *fd, struct sockaddr_can **saddrp, > > > + const void *arg) > > > +{ > > > + struct _rtdm_setsockaddr_args *setaddr, setaddr_buf; > > > + int ret = 0; > > > + > > > + if (!rtdm_fd_is_user(fd)) { > > > + setaddr = (struct _rtdm_setsockaddr_args *)arg; > > > + *saddrp = (struct sockaddr_can *)setaddr->addr; > > > + } > > > + > > > +#ifdef CONFIG_XENO_ARCH_SYS3264 > > > + if (rtdm_fd_is_compat(fd)) { > > > + struct compat_rtdm_setsockaddr_args csetaddr_buff; > > > + > > > + /* Copy argument structure from userspace */ > > > + ret = rtdm_safe_copy_from_user(fd, &csetaddr_buff, > > > + arg, sizeof(csetaddr_buff)); > > > + if (ret) > > > + return ret; > > > + > > > + /* Check size */ > > > + if (csetaddr_buff.addrlen != sizeof(**saddrp)) > > > > Below you use sizeof(sockaddr_can). For consistency reasons, both tests > > should use the same pattern. > > > > > + return -EINVAL; > > > + > > > + return rtdm_safe_copy_from_user(fd, *saddrp, > > > + compat_ptr(csetaddr_buff.addr), > > > + sizeof(struct sockaddr_can)); > > > + *saddrp = NULL; > > > + return 0; > > Dead code. The last two lines will never be executed. > > > > + } > > > +#endif > > > + > > > + if (rtdm_safe_copy_from_user(fd, &setaddr_buf, arg, > > > + sizeof(struct _rtdm_setsockaddr_args))) > > > + return -EFAULT; > > > + > > > + setaddr = &setaddr_buf; > > > + > > > + /* Check size */ > > > + if (setaddr->addrlen != sizeof(struct sockaddr_can)) > > > + return -EINVAL; > > > + > > > + return rtdm_safe_copy_from_user(fd, *saddrp, > > > + setaddr->addr, > > > + sizeof(struct sockaddr_can)); > > > + > > > + *saddrp = NULL; > > > + return 0; > > Dead code. > > > > +} > > > + > > > + > > > int rtcan_raw_socket(struct rtdm_fd *fd, int protocol) > > > { > > > /* Only protocol CAN_RAW is supported */ > > > @@ -408,46 +460,21 @@ static int rtcan_raw_setsockopt(struct rtdm_fd *fd, > > > int rtcan_raw_ioctl(struct rtdm_fd *fd, > > > unsigned int request, void *arg) > > > { > > > + struct sockaddr_can saddr, *saddrp = &saddr; > > > int ret = 0; > > > > > > switch (request) { > > > - 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; > > > > > > - setaddr = &setaddr_buf; > > > + COMPAT_CASE(_RTIOC_BIND): > > > + ret = rtcan_get_sockaddr(fd, &saddrp, arg); > > > + if (ret) > > > + return ret; > > > + if (saddrp == NULL) > > > + return -EFAULT; > > > + ret = rtcan_raw_bind(fd, saddrp); > > > + break; > > > > Seeing this weird indention (even when ignoring the legacy original > > indention of the target file before the change), I suspect the patch was > > mangled by your email client. > > > > > > > > - /* Check size */ > > > - if (setaddr->addrlen != sizeof(struct sockaddr_can)) > > > - return -EINVAL; > > > - > > > - /* Copy argument structure from userspace */ > > > - if (!rtdm_read_user_ok(fd, arg, > > > - sizeof(struct sockaddr_can)) || > > > - rtdm_copy_from_user(fd, &sockaddr_buf, setaddr->addr, > > > - sizeof(struct sockaddr_can))) > > > - return -EFAULT; > > > - sockaddr = &sockaddr_buf; > > > - } else { > > > - setaddr = (struct _rtdm_setsockaddr_args *)arg; > > > - sockaddr = (struct sockaddr_can *)setaddr->addr; > > > - } > > > - > > > - /* Now, all required data are in kernel space */ > > > - ret = rtcan_raw_bind(fd, sockaddr); > > > - > > > - break; > > > - } > > > - > > > - case _RTIOC_SETSOCKOPT: { > > > + COMPAT_CASE(_RTIOC_SETSOCKOPT): { > > > struct _rtdm_setsockopt_args *setopt; > > > struct _rtdm_setsockopt_args setopt_buf; > > > > > > @@ -532,7 +559,7 @@ ssize_t rtcan_raw_recvmsg(struct rtdm_fd *fd, > > > struct rtcan_socket *sock = rtdm_fd_to_private(fd); > > > struct sockaddr_can scan; > > > nanosecs_rel_t timeout; > > > - struct iovec *iov = (struct iovec *)msg->msg_iov; > > > + struct iovec iov_fast[RTDM_IOV_FASTMAX], *iov; > > > struct iovec iov_buf; > > > can_frame_t frame; > > > nanosecs_abs_t timestamp = 0; > > > @@ -584,6 +611,10 @@ ssize_t rtcan_raw_recvmsg(struct rtdm_fd *fd, > > > iov = &iov_buf; > > > } > > > > > > + ret = rtdm_get_iovec(fd, &iov, msg, iov_fast); > > > + if (ret) > > > + return ret; > > > + > > > /* Check size of buffer */ > > > if (iov->iov_len < sizeof(can_frame_t)) > > > return -EMSGSIZE; > > > @@ -768,7 +799,7 @@ ssize_t rtcan_raw_sendmsg(struct rtdm_fd *fd, > > > struct rtcan_socket *sock = rtdm_fd_to_private(fd); > > > struct sockaddr_can *scan = (struct sockaddr_can *)msg->msg_name; > > > struct sockaddr_can scan_buf; > > > - struct iovec *iov = (struct iovec *)msg->msg_iov; > > > + struct iovec iov_fast[RTDM_IOV_FASTMAX], *iov; > > > struct iovec iov_buf; > > > can_frame_t *frame; > > > can_frame_t frame_buf; > > > @@ -841,8 +872,12 @@ ssize_t rtcan_raw_sendmsg(struct rtdm_fd *fd, > > > iov = &iov_buf; > > > } > > > > > > + ret = rtdm_get_iovec(fd, &iov, msg, iov_fast); > > > + if (ret) > > > + return ret; > > > + > > > /* Check size of buffer */ > > > - if (iov->iov_len != sizeof(can_frame_t)) > > > + if (iov->iov_len != sizeof(can_frame_t)) > > > return -EMSGSIZE; > > > > > > frame = (can_frame_t *)iov->iov_base; > > > -- > > > 2.1.0 > > > > Thanks, > > Jan > > > -------------- next part -------------- A non-text attachment was scrubbed... Name: rtcan.patch.gz Type: application/gzip Size: 1730 bytes Desc: not available URL: <http://xenomai.org/pipermail/xenomai/attachments/20211105/f172ba0f/attachment.bin>
