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; > + } > +#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; > +} > + > + > 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 -- Siemens AG, T RDA IOT Corporate Competence Center Embedded Linux
