* [PATCH 1/1] drivers/can: 32-bit userspace app support for rtcan
@ 2022-01-26 1:32 Konstantin Smola
2022-02-15 8:26 ` Jan Kiszka
0 siblings, 1 reply; 5+ messages in thread
From: Konstantin Smola @ 2022-01-26 1:32 UTC (permalink / raw)
To: xenomai; +Cc: Konstantin Smola
kernel/drivers/can: 32-bit userspace app support for rtcan
Signed-off-by: Konstantin Smola <ksmola51@gmail.com>
---
kernel/drivers/can/rtcan_internal.h | 1 +
kernel/drivers/can/rtcan_raw.c | 102 +++++++++++++++++++++++-------------
2 files changed, 66 insertions(+), 37 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 dffc6e8..8aff03a 100644
--- a/kernel/drivers/can/rtcan_raw.c
+++ b/kernel/drivers/can/rtcan_raw.c
@@ -214,6 +214,51 @@ 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;
+ } else {
+#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)
{
@@ -408,46 +453,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;
- /* 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 +552,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 +604,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;
@@ -862,7 +886,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;
@@ -933,6 +957,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;
+
n = msg->msg_iovlen;
while (i < n) {
if (iov->iov_len < sizeof(can_frame_t)) {
--
2.1.0
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH 1/1] drivers/can: 32-bit userspace app support for rtcan
2022-01-26 1:32 [PATCH 1/1] drivers/can: 32-bit userspace app support for rtcan Konstantin Smola
@ 2022-02-15 8:26 ` Jan Kiszka
0 siblings, 0 replies; 5+ messages in thread
From: Jan Kiszka @ 2022-02-15 8:26 UTC (permalink / raw)
To: Konstantin Smola, xenomai
On 26.01.22 02:32, Konstantin Smola via Xenomai wrote:
> kernel/drivers/can: 32-bit userspace app support for rtcan
Not an optimal commit message - just repeating the subject line. How
about this?
"The CAN interface was lacking 32-bit support for iovec processing on
sendmsg/recvmsg as well as for bind and setsockopt handling."
> Signed-off-by: Konstantin Smola <ksmola51@gmail.com>
> ---
> kernel/drivers/can/rtcan_internal.h | 1 +
> kernel/drivers/can/rtcan_raw.c | 102 +++++++++++++++++++++++-------------
> 2 files changed, 66 insertions(+), 37 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 dffc6e8..8aff03a 100644
> --- a/kernel/drivers/can/rtcan_raw.c
> +++ b/kernel/drivers/can/rtcan_raw.c
> @@ -214,6 +214,51 @@ 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;
If you take this path...
> + } else {
> +#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));
...you don't need the copy, do you? In fact, it will even fail on modern
kernels if the source is kernel memory, not user memory.
> +}
>
> int rtcan_raw_socket(struct rtdm_fd *fd, int protocol)
> {
> @@ -408,46 +453,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;
>
> - /* 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 +552,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 +604,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;
> @@ -862,7 +886,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;
> @@ -933,6 +957,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;
> +
> n = msg->msg_iovlen;
> while (i < n) {
> if (iov->iov_len < sizeof(can_frame_t)) {
Indention is off, even in terms of that legacy indention format the file
uses (needs a mass-reformatting, just like RTnet). Please make sure that
the code is at least readable (tabs are 8 spaces).
Thanks,
Jan
--
Siemens AG, Technology
Competence Center Embedded Linux
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 1/1] drivers/can: 32-bit userspace app support for rtcan
2021-11-08 10:29 ` Jan Kiszka
@ 2021-12-01 6:11 ` Jan Kiszka
0 siblings, 0 replies; 5+ messages in thread
From: Jan Kiszka @ 2021-12-01 6:11 UTC (permalink / raw)
To: C Smith, xenomai
On 08.11.21 11:29, Jan Kiszka via Xenomai wrote:
> Commit message is missing.
>
> On 05.11.21 20:59, C Smith via Xenomai wrote:
>> Signed-off-by: C Smith <csmithquestions@gmail.com>
>> ---
>> 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;
>
> In the !is_user case...
>
>> + }
>> +
>> +#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;
>
> ...this whole block must not be taken. See old code.
>
>> +
>> + /* 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;
>
> Wrong indentions, mixing spaced and tabs. Let's not make things worse
> than they are (the file needs a mass-conversion to kernel style at some
> point).
>
>> + 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;
>>
>
> We can sort this out and merge after the 3.2 release, possibly also
> backporting things to 3.1.
>
> Jan
>
Ping on this - just remembered as I was looking at the other CAN patches.
Jan
--
Siemens AG, T RDA IOT
Corporate Competence Center Embedded Linux
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 1/1] drivers/can: 32-bit userspace app support for rtcan
2021-11-05 19:59 C Smith
@ 2021-11-08 10:29 ` Jan Kiszka
2021-12-01 6:11 ` Jan Kiszka
0 siblings, 1 reply; 5+ messages in thread
From: Jan Kiszka @ 2021-11-08 10:29 UTC (permalink / raw)
To: C Smith, xenomai
Commit message is missing.
On 05.11.21 20:59, C Smith via Xenomai wrote:
> Signed-off-by: C Smith <csmithquestions@gmail.com>
> ---
> 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;
In the !is_user case...
> + }
> +
> +#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;
...this whole block must not be taken. See old code.
> +
> + /* 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;
Wrong indentions, mixing spaced and tabs. Let's not make things worse
than they are (the file needs a mass-conversion to kernel style at some
point).
> + 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;
>
We can sort this out and merge after the 3.2 release, possibly also
backporting things to 3.1.
Jan
--
Siemens AG, T RDA IOT
Corporate Competence Center Embedded Linux
^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH 1/1] drivers/can: 32-bit userspace app support for rtcan
@ 2021-11-05 19:59 C Smith
2021-11-08 10:29 ` Jan Kiszka
0 siblings, 1 reply; 5+ messages in thread
From: C Smith @ 2021-11-05 19:59 UTC (permalink / raw)
To: xenomai
Signed-off-by: C Smith <csmithquestions@gmail.com>
---
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
^ permalink raw reply related [flat|nested] 5+ messages in thread
end of thread, other threads:[~2022-02-15 8:26 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-01-26 1:32 [PATCH 1/1] drivers/can: 32-bit userspace app support for rtcan Konstantin Smola
2022-02-15 8:26 ` Jan Kiszka
-- strict thread matches above, loose matches on Subject: below --
2021-11-05 19:59 C Smith
2021-11-08 10:29 ` Jan Kiszka
2021-12-01 6:11 ` Jan Kiszka
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.