rtcansend 32-bit

rtcansend 32-bit

[C Smith]

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

Re: rtcansend 32-bit

[Jan Kiszka]

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

Re: rtcansend 32-bit

[C Smith]

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
>

Re: rtcansend 32-bit

[Jan Kiszka]

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

Re: rtcansend 32-bit

[Jan Kiszka]

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

Re: rtcansend 32-bit

[Bezdeka, Florian]

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
> 

Re: rtcansend 32-bit

[C Smith]

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
> >
>
>

Re: rtcansend 32-bit

[Bezdeka, Florian]

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
> > > 
> > 

Re: rtcansend 32-bit

[C Smith]

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

Re: rtcansend 32-bit

[Jan Kiszka]

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

Re: rtcansend 32-bit

[Bezdeka, Florian]

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
> 

Re: rtcansend 32-bit

[C Smith]

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>

Re: rtcansend 32-bit

[Jan Kiszka]

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