[Xenomai-core] Re: iovec overwriting by CAN stack

[Jan Kiszka <jan.kiszka@domain.hid>]

[-- Attachment #1: Type: text/plain, Size: 2855 bytes --]

Sebastian Smolorz wrote:
> Jan Kiszka wrote:
>> Sebastian Smolorz wrote:
>>> Jan Kiszka wrote:
>>>> Wolfgang Grandegger wrote:
>>>>> Jan Kiszka wrote:
>>>>>> Hi Wolfgang,
>>>>>>
>>>>>> it's late, so I may have misread somecode, but don't you "update" the
>>>>>> iovec descriptors a user passes on send/recvmsg on return (namely
>>>>>> iovec_len)? I received some complaints about this /wrt to in-kernel
>>>>>> CAN stack usage.
>>>>> It's done here:
>>>>>
>>>>> http://www.rts.uni-hannover.de/xenomai/lxr/source/ksrc/drivers/can/rtca
>>>>> n_ raw.c?v=SVN-trunk#881
>>>>>
>>>>> http://www.rts.uni-hannover.de/xenomai/lxr/source/ksrc/drivers/can/rtca
>>>>> n_ raw.c?v=SVN-trunk#734
>>>>>
>>>>>
>>>>> I may have missed something. What are the real complaints? Is there a
>>>>> test program?
>>>> Not yet (will commit the related patch to our RACK likely later today).
>>>> It's simply sending frames while re-using the msg+iovec structs in a
>>>> loop.
>>>>
>>>>>> I always considered the same well-know behaviour of RTnet a bug, but
>>>>>> now I found your code is doing this systematically, also for user
>>>>>> space callers. Is this behaviour undefined or even required according
>>>>>> POSIX or whatever?
>>>>> I don't know, it's Sebastian's Kode.
>>>> Hmm, hope he will not say that he imitated RTnet...
>>> Rather an imitation of the Linux kernel's behaviour. The memcpy_toiovec()
>>> and memcpy_fromiovec() functions [1] also modify the original iovec.
>>>
>>>
>>> [1] http://lxr.free-electrons.com/source/net/core/iovec.c
>> But that's the kernel's private (and user-safe) copy. I failed to find
>> it writing that back to user space.
> 
> Hm. Ad hoc, I don't recall why the userspace's iovec is also updated. Perhaps 
> my intention was to make the use of rtcan_raw_sendmsg consistent for kernel 
> and userspace RT-Tasks so that both can rely on finding modified iovecs after 
> calling sendmsg/recvmsg. But I'm not absolutely sure if there isn't a better 
> explanation for this. I will try to find the reason in my backup memory. ;-)
> 

So, unless we find some reason why we _must_ modify user's iovec, either
for send or receive, I would say: kill the user copy-back and work in
both case (kernel / user space caller) on the local copy of iovec (if we
still need to modify it all then).

Jan


PS: I just committed this fix (only a problem for in-kernel usage)

--- ksrc/drivers/can/rtcan_raw.c        (Revision 2340)
+++ ksrc/drivers/can/rtcan_raw.c        (Arbeitskopie)
@@ -334,7 +334,7 @@ static int rtcan_raw_setsockopt(struct r
                    return -EFAULT;
                }
            } else
-               memcpy(flist, so->optval, so->optlen);
+               memcpy(flist->flist, so->optval, so->optlen);
            flist->flistlen = flistlen;
        }


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 250 bytes --]