* rtcansend 32-bit @ 2021-11-02 18:57 C Smith 2021-11-02 19:11 ` Jan Kiszka 0 siblings, 1 reply; 13+ messages in thread From: C Smith @ 2021-11-02 18:57 UTC (permalink / raw) To: Xenomai List I ran into a problem wherein my real-time Xenomai 32-bit app fails on the socket operations of the 64-bit CAN driver. My real-time userspace app is Cobalt x86, compiled -m32. When I try the Xenomai rtcansend.c sample app compiled 32-bit, I get the same error : [root@pc can]# /usr/xenomai/bin/rtcansend rtcan0 --verbose --identifier=0x123 0xde 0xad send: Message too long Looking at rtcansend.c. The call to sendto is failing and returns 'Message too long' here: ret = sendto(s, (void *)&frame, sizeof(can_frame_t), 0, (struct sockaddr *)&to_addr, sizeof(to_addr)); Here is how I configured Xenomai for 32-bit when I built it: ./configure --host=i686-linux CFLAGS="-m32 -D_FILE_OFFSET_BITS=64" LDFLAGS=-m32 host_alias=i686-linux But note that if I compile both Xenomai and rtcansend.c 64-bit, everything works fine. So how can 32-bit Xeno apps use the 64-bit CAN driver ? thanks, -C Smith ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: rtcansend 32-bit 2021-11-02 18:57 rtcansend 32-bit C Smith @ 2021-11-02 19:11 ` Jan Kiszka 2021-11-02 22:57 ` C Smith 0 siblings, 1 reply; 13+ messages in thread From: Jan Kiszka @ 2021-11-02 19:11 UTC (permalink / raw) To: C Smith, Xenomai List On 02.11.21 19:57, C Smith via Xenomai wrote: > I ran into a problem wherein my real-time Xenomai 32-bit app > fails on the socket operations of the 64-bit CAN driver. > My real-time userspace app is Cobalt x86, compiled -m32. > > When I try the Xenomai rtcansend.c sample app compiled 32-bit, I get the > same error : > > [root@pc can]# /usr/xenomai/bin/rtcansend rtcan0 --verbose > --identifier=0x123 0xde 0xad > send: Message too long > > Looking at rtcansend.c. The call to sendto is failing and returns 'Message > too long' here: > ret = sendto(s, (void *)&frame, sizeof(can_frame_t), 0, > (struct sockaddr *)&to_addr, sizeof(to_addr)); > > Here is how I configured Xenomai for 32-bit when I built it: > ./configure --host=i686-linux CFLAGS="-m32 -D_FILE_OFFSET_BITS=64" > LDFLAGS=-m32 host_alias=i686-linux > > But note that if I compile both Xenomai and rtcansend.c 64-bit, everything > works fine. > > So how can 32-bit Xeno apps use the 64-bit CAN driver ? > We possibly have compat ABI issue here: The Xenomai core can handle 32-bit calls of sendto (sendmsg internally) into 64-bit kernel, but maybe there is something beyond that interpreted by the CAN driver that is not aware of 32 vs. 64 bit userland. I suspect sizeof(can_frame_t) varies, though I don't see why yet. Could you debug that, instrument kernel/drivers/can/rtcan_raw.c as well as the userspace code? The other reason for EMSGSIZE would be msg_iovlen != 1, but that would mean we have an issue in the generic compat sendmsg code, and that is less likely. Jan -- Siemens AG, T RDA IOT Corporate Competence Center Embedded Linux ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: rtcansend 32-bit 2021-11-02 19:11 ` Jan Kiszka @ 2021-11-02 22:57 ` C Smith 2021-11-03 6:59 ` Jan Kiszka 0 siblings, 1 reply; 13+ messages in thread From: C Smith @ 2021-11-02 22:57 UTC (permalink / raw) To: Xenomai List I added some printf/printk to rtcansend.c as well as rtcan_raw.c: rtcan_raw.c: /* Check size of buffer */ if (iov->iov_len != sizeof(can_frame_t)) { printk("rtcan_raw.c, 850: sizeof(can_frame_t): %ld\n", sizeof(can_frame_t)); printk("rtcan_raw.c, 852: iov->iov_len: %ld\n", iov->iov_len); return -EMSGSIZE; } when running rtcansend (32-bit compile, which fails with EMSGSIZE): [root@pc can]# /usr/xenomai/bin/rtcansend rtcan0 -s 0xde 0xad sizeof(can_frame_t): 16 send: Message too long [root@pc can]# dmesg [11275.197125] rtcan_raw.c, 850: sizeof(can_frame_t): 16 [11275.197133] rtcan_raw.c, 852: iov->iov_len: 34494267600 when running rtcansend (64-bit compile, sends out can msg OK): [root@pc can]# /usr/xenomai/bin/rtcansend rtcan0 -s 0xde 0xad sizeof(can_frame_t): 16 [root@pc can]# dmesg [12476.571032] rtcan_raw.c, 850: sizeof(can_frame_t): 16 [12476.571040] rtcan_raw.c, 852: iov->iov_len: 16 It looks like the struct user_msghdr *msg passed into rtcan_raw_sendmsg() is corrupt. I'm using Xenomai 3.1, with kernel 4.19.989 x86_64 -C Smith On Tue, Nov 2, 2021 at 12:11 PM Jan Kiszka <jan.kiszka@siemens.com> wrote: > On 02.11.21 19:57, C Smith via Xenomai wrote: > > I ran into a problem wherein my real-time Xenomai 32-bit app > > fails on the socket operations of the 64-bit CAN driver. > > My real-time userspace app is Cobalt x86, compiled -m32. > > > > When I try the Xenomai rtcansend.c sample app compiled 32-bit, I get the > > same error : > > > > [root@pc can]# /usr/xenomai/bin/rtcansend rtcan0 --verbose > > --identifier=0x123 0xde 0xad > > send: Message too long > > > > Looking at rtcansend.c. The call to sendto is failing and returns > 'Message > > too long' here: > > ret = sendto(s, (void *)&frame, sizeof(can_frame_t), 0, > > (struct sockaddr *)&to_addr, sizeof(to_addr)); > > > > Here is how I configured Xenomai for 32-bit when I built it: > > ./configure --host=i686-linux CFLAGS="-m32 -D_FILE_OFFSET_BITS=64" > > LDFLAGS=-m32 host_alias=i686-linux > > > > But note that if I compile both Xenomai and rtcansend.c 64-bit, > everything > > works fine. > > > > So how can 32-bit Xeno apps use the 64-bit CAN driver ? > > > > We possibly have compat ABI issue here: The Xenomai core can handle > 32-bit calls of sendto (sendmsg internally) into 64-bit kernel, but > maybe there is something beyond that interpreted by the CAN driver that > is not aware of 32 vs. 64 bit userland. > > I suspect sizeof(can_frame_t) varies, though I don't see why yet. Could > you debug that, instrument kernel/drivers/can/rtcan_raw.c as well as the > userspace code? > > The other reason for EMSGSIZE would be msg_iovlen != 1, but that would > mean we have an issue in the generic compat sendmsg code, and that is > less likely. > > Jan > > -- > Siemens AG, T RDA IOT > Corporate Competence Center Embedded Linux > ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: rtcansend 32-bit 2021-11-02 22:57 ` C Smith @ 2021-11-03 6:59 ` Jan Kiszka 2021-11-03 10:46 ` Jan Kiszka 0 siblings, 1 reply; 13+ messages in thread From: Jan Kiszka @ 2021-11-03 6:59 UTC (permalink / raw) To: C Smith, Xenomai List On 02.11.21 23:57, C Smith via Xenomai wrote: > I added some printf/printk to rtcansend.c as well as rtcan_raw.c: > > rtcan_raw.c: > /* Check size of buffer */ > if (iov->iov_len != sizeof(can_frame_t)) { > printk("rtcan_raw.c, 850: sizeof(can_frame_t): %ld\n", > sizeof(can_frame_t)); > printk("rtcan_raw.c, 852: iov->iov_len: %ld\n", > iov->iov_len); > return -EMSGSIZE; > } > > when running rtcansend (32-bit compile, which fails with EMSGSIZE): > [root@pc can]# /usr/xenomai/bin/rtcansend rtcan0 -s 0xde 0xad > sizeof(can_frame_t): 16 > send: Message too long > > [root@pc can]# dmesg > [11275.197125] rtcan_raw.c, 850: sizeof(can_frame_t): 16 > [11275.197133] rtcan_raw.c, 852: iov->iov_len: 34494267600 > > when running rtcansend (64-bit compile, sends out can msg OK): > [root@pc can]# /usr/xenomai/bin/rtcansend rtcan0 -s 0xde 0xad > sizeof(can_frame_t): 16 > > [root@pc can]# dmesg > [12476.571032] rtcan_raw.c, 850: sizeof(can_frame_t): 16 > [12476.571040] rtcan_raw.c, 852: iov->iov_len: 16 > > It looks like the struct user_msghdr *msg passed into rtcan_raw_sendmsg() > is corrupt. > I'm using Xenomai 3.1, with kernel 4.19.989 x86_64 > -C Smith OK, my guess was wrong. Let me see where we corrupt this. Brings https://gitlab.com/Xenomai/xenomai-hacker-space/-/issues/21 into memory... Jan -- Siemens AG, T RDA IOT Corporate Competence Center Embedded Linux ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: rtcansend 32-bit 2021-11-03 6:59 ` Jan Kiszka @ 2021-11-03 10:46 ` Jan Kiszka 2021-11-03 11:09 ` Bezdeka, Florian 0 siblings, 1 reply; 13+ messages in thread From: Jan Kiszka @ 2021-11-03 10:46 UTC (permalink / raw) To: C Smith, Xenomai List On 03.11.21 07:59, Jan Kiszka wrote: > On 02.11.21 23:57, C Smith via Xenomai wrote: >> I added some printf/printk to rtcansend.c as well as rtcan_raw.c: >> >> rtcan_raw.c: >> /* Check size of buffer */ >> if (iov->iov_len != sizeof(can_frame_t)) { >> printk("rtcan_raw.c, 850: sizeof(can_frame_t): %ld\n", >> sizeof(can_frame_t)); >> printk("rtcan_raw.c, 852: iov->iov_len: %ld\n", >> iov->iov_len); >> return -EMSGSIZE; >> } >> >> when running rtcansend (32-bit compile, which fails with EMSGSIZE): >> [root@pc can]# /usr/xenomai/bin/rtcansend rtcan0 -s 0xde 0xad >> sizeof(can_frame_t): 16 >> send: Message too long >> >> [root@pc can]# dmesg >> [11275.197125] rtcan_raw.c, 850: sizeof(can_frame_t): 16 >> [11275.197133] rtcan_raw.c, 852: iov->iov_len: 34494267600 >> >> when running rtcansend (64-bit compile, sends out can msg OK): >> [root@pc can]# /usr/xenomai/bin/rtcansend rtcan0 -s 0xde 0xad >> sizeof(can_frame_t): 16 >> >> [root@pc can]# dmesg >> [12476.571032] rtcan_raw.c, 850: sizeof(can_frame_t): 16 >> [12476.571040] rtcan_raw.c, 852: iov->iov_len: 16 >> >> It looks like the struct user_msghdr *msg passed into rtcan_raw_sendmsg() >> is corrupt. >> I'm using Xenomai 3.1, with kernel 4.19.989 x86_64 >> -C Smith > > OK, my guess was wrong. Let me see where we corrupt this. > > Brings https://gitlab.com/Xenomai/xenomai-hacker-space/-/issues/21 into > memory... > Found it: We are lacking use of rtdm_get_iovec in rtcan - in contrast to RTnet (see e.g. rt_packet_sendmsg). Would you feel like looking into such a change? Jan -- Siemens AG, T RDA IOT Corporate Competence Center Embedded Linux ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: rtcansend 32-bit 2021-11-03 10:46 ` Jan Kiszka @ 2021-11-03 11:09 ` Bezdeka, Florian 2021-11-04 6:49 ` C Smith 0 siblings, 1 reply; 13+ messages in thread From: Bezdeka, Florian @ 2021-11-03 11:09 UTC (permalink / raw) To: xenomai, jan.kiszka, csmithquestions On Wed, 2021-11-03 at 11:46 +0100, Jan Kiszka via Xenomai wrote: > On 03.11.21 07:59, Jan Kiszka wrote: > > On 02.11.21 23:57, C Smith via Xenomai wrote: > > > I added some printf/printk to rtcansend.c as well as rtcan_raw.c: > > > > > > rtcan_raw.c: > > > /* Check size of buffer */ > > > if (iov->iov_len != sizeof(can_frame_t)) { > > > printk("rtcan_raw.c, 850: sizeof(can_frame_t): %ld\n", > > > sizeof(can_frame_t)); > > > printk("rtcan_raw.c, 852: iov->iov_len: %ld\n", > > > iov->iov_len); > > > return -EMSGSIZE; > > > } > > > > > > when running rtcansend (32-bit compile, which fails with EMSGSIZE): > > > [root@pc can]# /usr/xenomai/bin/rtcansend rtcan0 -s 0xde 0xad > > > sizeof(can_frame_t): 16 > > > send: Message too long > > > > > > [root@pc can]# dmesg > > > [11275.197125] rtcan_raw.c, 850: sizeof(can_frame_t): 16 > > > [11275.197133] rtcan_raw.c, 852: iov->iov_len: 34494267600 > > > > > > when running rtcansend (64-bit compile, sends out can msg OK): > > > [root@pc can]# /usr/xenomai/bin/rtcansend rtcan0 -s 0xde 0xad > > > sizeof(can_frame_t): 16 > > > > > > [root@pc can]# dmesg > > > [12476.571032] rtcan_raw.c, 850: sizeof(can_frame_t): 16 > > > [12476.571040] rtcan_raw.c, 852: iov->iov_len: 16 > > > > > > It looks like the struct user_msghdr *msg passed into rtcan_raw_sendmsg() > > > is corrupt. > > > I'm using Xenomai 3.1, with kernel 4.19.989 x86_64 > > > -C Smith > > > > OK, my guess was wrong. Let me see where we corrupt this. > > > > Brings https://gitlab.com/Xenomai/xenomai-hacker-space/-/issues/21 into > > memory... > > > > Found it: We are lacking use of rtdm_get_iovec in rtcan - in contrast to > RTnet (see e.g. rt_packet_sendmsg). Would you feel like looking into > such a change? Just a note: rtcan_raw_sendmsg() and rtcan_raw_recvmsg() are both affected. Both should be using rtdm_get_iovec(). > > Jan > ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: rtcansend 32-bit 2021-11-03 11:09 ` Bezdeka, Florian @ 2021-11-04 6:49 ` C Smith 2021-11-04 8:05 ` Bezdeka, Florian 0 siblings, 1 reply; 13+ messages in thread From: C Smith @ 2021-11-04 6:49 UTC (permalink / raw) To: Bezdeka, Florian; +Cc: xenomai, jan.kiszka I was able to make the CAN transmit work successfully from my 32 bit compile of rtcansend.c, by adding rtdm_get_iovec() to the code. I'll submit a patch once both transmit and receive are working, but I'm still having trouble receiving data in the rtcanrecv app. I added rtdm_get_iovec() to rtcan_raw_recvmsg() so that may ultimately work but the driver is already giving up during the bind. I get this error: [root@pc can]# /usr/xenomai/bin/rtcanrecv rtcan0 -v interface rtcan0 s=3, ifr_name=rtcan0 bind: Invalid argument Cleaning up... Here's the ioctl with extra printk()s, which executes during the bind: int rtcan_raw_ioctl(struct rtdm_fd *fd,unsigned int request, void *arg) { int ret = 0; switch (request) { 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 */ printk("rtcan_raw.c, 421: rtcan_raw_ioctl\n"); if (rtdm_safe_copy_from_user(fd, &setaddr_buf, arg, sizeof(struct _rtdm_setsockaddr_args))) return -EFAULT; setaddr = &setaddr_buf; printk("rtcan_raw.c, 427: rtcan_raw_ioctl\n"); printk("rtcan_raw.c, 428: setaddr->addrlen: %d\n", setaddr->addrlen); /* Check size */ if (setaddr->addrlen != sizeof(struct sockaddr_can)) return -EINVAL; The resultant print statements in dmesg : [27177.480980] rtcan_raw.c, 421: rtcan_raw_ioctl [27177.480987] rtcan_raw.c, 427: rtcan_raw_ioctl [27177.480994] rtcan_raw.c, 428: setaddr->addrlen: -6084360 Do you have any idea why addrlen is corrupt ? Thanks, -C Smith On Wed, Nov 3, 2021 at 4:09 AM Bezdeka, Florian <florian.bezdeka@siemens.com> wrote: > On Wed, 2021-11-03 at 11:46 +0100, Jan Kiszka via Xenomai wrote: > > On 03.11.21 07:59, Jan Kiszka wrote: > > > On 02.11.21 23:57, C Smith via Xenomai wrote: > > > > I added some printf/printk to rtcansend.c as well as rtcan_raw.c: > > > > > > > > rtcan_raw.c: > > > > /* Check size of buffer */ > > > > if (iov->iov_len != sizeof(can_frame_t)) { > > > > printk("rtcan_raw.c, 850: sizeof(can_frame_t): %ld\n", > > > > sizeof(can_frame_t)); > > > > printk("rtcan_raw.c, 852: iov->iov_len: %ld\n", > > > > iov->iov_len); > > > > return -EMSGSIZE; > > > > } > > > > > > > > when running rtcansend (32-bit compile, which fails with EMSGSIZE): > > > > [root@pc can]# /usr/xenomai/bin/rtcansend rtcan0 -s 0xde > 0xad > > > > sizeof(can_frame_t): 16 > > > > send: Message too long > > > > > > > > [root@pc can]# dmesg > > > > [11275.197125] rtcan_raw.c, 850: sizeof(can_frame_t): 16 > > > > [11275.197133] rtcan_raw.c, 852: iov->iov_len: 34494267600 > > > > > > > > when running rtcansend (64-bit compile, sends out can msg OK): > > > > [root@pc can]# /usr/xenomai/bin/rtcansend rtcan0 -s 0xde > 0xad > > > > sizeof(can_frame_t): 16 > > > > > > > > [root@pc can]# dmesg > > > > [12476.571032] rtcan_raw.c, 850: sizeof(can_frame_t): 16 > > > > [12476.571040] rtcan_raw.c, 852: iov->iov_len: 16 > > > > > > > > It looks like the struct user_msghdr *msg passed into > rtcan_raw_sendmsg() > > > > is corrupt. > > > > I'm using Xenomai 3.1, with kernel 4.19.989 x86_64 > > > > -C Smith > > > > > > OK, my guess was wrong. Let me see where we corrupt this. > > > > > > Brings https://gitlab.com/Xenomai/xenomai-hacker-space/-/issues/21 > into > > > memory... > > > > > > > Found it: We are lacking use of rtdm_get_iovec in rtcan - in contrast to > > RTnet (see e.g. rt_packet_sendmsg). Would you feel like looking into > > such a change? > > Just a note: rtcan_raw_sendmsg() and rtcan_raw_recvmsg() are both > affected. Both should be using rtdm_get_iovec(). > > > > > Jan > > > > ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: rtcansend 32-bit 2021-11-04 6:49 ` C Smith @ 2021-11-04 8:05 ` Bezdeka, Florian 2021-11-05 7:09 ` C Smith 0 siblings, 1 reply; 13+ messages in thread From: Bezdeka, Florian @ 2021-11-04 8:05 UTC (permalink / raw) To: csmithquestions; +Cc: xenomai, jan.kiszka On Wed, 2021-11-03 at 23:49 -0700, C Smith wrote: > I was able to make the CAN transmit work successfully from my 32 bit > compile of rtcansend.c, by adding rtdm_get_iovec() to the code. I'll > submit a patch once both transmit and receive are working, but I'm > still having trouble receiving data in the rtcanrecv app. I added > rtdm_get_iovec() to rtcan_raw_recvmsg() so that may ultimately work > but the driver is already giving up during the bind. I get this > error: > > [root@pc can]# /usr/xenomai/bin/rtcanrecv rtcan0 -v > interface rtcan0 > s=3, ifr_name=rtcan0 > bind: Invalid argument > Cleaning up... > > Here's the ioctl with extra printk()s, which executes during the > bind: > > int rtcan_raw_ioctl(struct rtdm_fd *fd,unsigned int request, void > *arg) > { > int ret = 0; > > switch (request) { > 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 */ > printk("rtcan_raw.c, 421: rtcan_raw_ioctl\n"); > > if (rtdm_safe_copy_from_user(fd, &setaddr_buf, arg, > sizeof(struct _rtdm_setsockaddr_args))) > return -EFAULT; That should be the source of your problem. It looks like rtcan never had a compat interface. (Jan should confirm first...) You have a 64 bit kernel, so the pointer width (in struct _rtdm_setsockaddr_args) is considered to be 8 bytes but userspace only provided 4. The copy operation still succeeds, but reading from this struct is now broken because of wrong offsets. Reading struct _rtdm_setsockaddr_args has be wrapped into a helper similar to rtdm_get_iovec(). > > setaddr = &setaddr_buf; > printk("rtcan_raw.c, 427: rtcan_raw_ioctl\n"); > printk("rtcan_raw.c, 428: setaddr->addrlen: %d\n", setaddr- > >addrlen); > > /* Check size */ > if (setaddr->addrlen != sizeof(struct sockaddr_can)) > return -EINVAL; > > The resultant print statements in dmesg : > [27177.480980] rtcan_raw.c, 421: rtcan_raw_ioctl > [27177.480987] rtcan_raw.c, 427: rtcan_raw_ioctl > [27177.480994] rtcan_raw.c, 428: setaddr->addrlen: -6084360 > > Do you have any idea why addrlen is corrupt ? > Thanks, -C Smith > > On Wed, Nov 3, 2021 at 4:09 AM Bezdeka, Florian > <florian.bezdeka@siemens.com> wrote: > > On Wed, 2021-11-03 at 11:46 +0100, Jan Kiszka via Xenomai wrote: > > > On 03.11.21 07:59, Jan Kiszka wrote: > > > > On 02.11.21 23:57, C Smith via Xenomai wrote: > > > > > I added some printf/printk to rtcansend.c as well as > > rtcan_raw.c: > > > > > > > > > > rtcan_raw.c: > > > > > /* Check size of buffer */ > > > > > if (iov->iov_len != sizeof(can_frame_t)) { > > > > > printk("rtcan_raw.c, 850: sizeof(can_frame_t): > > %ld\n", > > > > > sizeof(can_frame_t)); > > > > > printk("rtcan_raw.c, 852: iov->iov_len: > > > > > %ld\n", > > > > > iov->iov_len); > > > > > return -EMSGSIZE; > > > > > } > > > > > > > > > > when running rtcansend (32-bit compile, which fails with > > EMSGSIZE): > > > > > [root@pc can]# /usr/xenomai/bin/rtcansend rtcan0 -s > > 0xde 0xad > > > > > sizeof(can_frame_t): 16 > > > > > send: Message too long > > > > > > > > > > [root@pc can]# dmesg > > > > > [11275.197125] rtcan_raw.c, 850: > > > > > sizeof(can_frame_t): > > 16 > > > > > [11275.197133] rtcan_raw.c, 852: iov->iov_len: > > 34494267600 > > > > > > > > > > when running rtcansend (64-bit compile, sends out can msg > > > > > OK): > > > > > [root@pc can]# /usr/xenomai/bin/rtcansend rtcan0 -s > > 0xde 0xad > > > > > sizeof(can_frame_t): 16 > > > > > > > > > > [root@pc can]# dmesg > > > > > [12476.571032] rtcan_raw.c, 850: > > > > > sizeof(can_frame_t): > > 16 > > > > > [12476.571040] rtcan_raw.c, 852: iov->iov_len: 16 > > > > > > > > > > It looks like the struct user_msghdr *msg passed into > > rtcan_raw_sendmsg() > > > > > is corrupt. > > > > > I'm using Xenomai 3.1, with kernel 4.19.989 x86_64 > > > > > -C Smith > > > > > > > > OK, my guess was wrong. Let me see where we corrupt this. > > > > > > > > Brings > > https://gitlab.com/Xenomai/xenomai-hacker-space/-/issues/21 into > > > > memory... > > > > > > > > > > Found it: We are lacking use of rtdm_get_iovec in rtcan - in > > contrast to > > > RTnet (see e.g. rt_packet_sendmsg). Would you feel like looking > > into > > > such a change? > > > > Just a note: rtcan_raw_sendmsg() and rtcan_raw_recvmsg() are both > > affected. Both should be using rtdm_get_iovec(). > > > > > > > > Jan > > > > > ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: rtcansend 32-bit 2021-11-04 8:05 ` Bezdeka, Florian @ 2021-11-05 7:09 ` C Smith 2021-11-05 8:14 ` Jan Kiszka 0 siblings, 1 reply; 13+ messages in thread From: C Smith @ 2021-11-05 7:09 UTC (permalink / raw) To: Bezdeka, Florian; +Cc: xenomai, jan.kiszka 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. Signed-off-by: C Smith <csmithquestions@gmail.com> --- kernel/drivers/can/rtcan_internal.h | 3 + kernel/drivers/can/rtcan_raw.c | 111 ++++++++++++++++++++++++------------ 2 files changed, 76 insertions(+), 38 deletions(-) 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) + #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)) + 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; - /* 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 ^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: rtcansend 32-bit 2021-11-05 7:09 ` C Smith @ 2021-11-05 8:14 ` Jan Kiszka 2021-11-05 8:25 ` Bezdeka, Florian 0 siblings, 1 reply; 13+ messages in thread From: Jan Kiszka @ 2021-11-05 8:14 UTC (permalink / raw) To: C Smith, Bezdeka, Florian; +Cc: xenomai 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 <csmithquestions@gmail.com > <mailto:csmithquestions@gmail.com>> > --- ...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 ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: rtcansend 32-bit 2021-11-05 8:14 ` Jan Kiszka @ 2021-11-05 8:25 ` Bezdeka, Florian 2021-11-05 18:14 ` C Smith 0 siblings, 1 reply; 13+ messages in thread From: Bezdeka, Florian @ 2021-11-05 8:25 UTC (permalink / raw) To: jan.kiszka, csmithquestions; +Cc: xenomai 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 <csmithquestions@gmail.com > > <mailto:csmithquestions@gmail.com>> > > --- > > ...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 > ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: rtcansend 32-bit 2021-11-05 8:25 ` Bezdeka, Florian @ 2021-11-05 18:14 ` C Smith 2021-11-05 18:21 ` Jan Kiszka 0 siblings, 1 reply; 13+ messages in thread From: C Smith @ 2021-11-05 18:14 UTC (permalink / raw) To: xenomai, Bezdeka, Florian, jan.kiszka 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 <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 On Fri, Nov 5, 2021 at 1:25 AM Bezdeka, Florian <florian.bezdeka@siemens.com> 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 <csmithquestions@gmail.com > > > <mailto:csmithquestions@gmail.com>> > > > --- > > > > ...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> ^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: rtcansend 32-bit 2021-11-05 18:14 ` C Smith @ 2021-11-05 18:21 ` Jan Kiszka 0 siblings, 0 replies; 13+ messages in thread From: Jan Kiszka @ 2021-11-05 18:21 UTC (permalink / raw) To: C Smith, xenomai, Bezdeka, Florian On 05.11.21 19:14, C Smith wrote: > 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. > Please use git send-email against gmail's SMTP server. The attached .gz version is also incomplete. Buildroot has a nicely detailed description how to get that ball rolling (just replace addresses): https://buildroot.org/downloads/manual/manual.html#submitting-patches Jan -- Siemens AG, T RDA IOT Corporate Competence Center Embedded Linux ^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2021-11-05 18:21 UTC | newest] Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-11-02 18:57 rtcansend 32-bit C Smith 2021-11-02 19:11 ` Jan Kiszka 2021-11-02 22:57 ` C Smith 2021-11-03 6:59 ` Jan Kiszka 2021-11-03 10:46 ` Jan Kiszka 2021-11-03 11:09 ` Bezdeka, Florian 2021-11-04 6:49 ` C Smith 2021-11-04 8:05 ` Bezdeka, Florian 2021-11-05 7:09 ` C Smith 2021-11-05 8:14 ` Jan Kiszka 2021-11-05 8:25 ` Bezdeka, Florian 2021-11-05 18:14 ` C Smith 2021-11-05 18:21 ` 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.