Xenomai recvmsg cmsg patch

Current implementation of Xenomai's recvmsg syscall doesn't handle msghdr fields (msg_control, msg_controllen) correctly. It's necessary to change msg_controllen to the number of bytes written to the buffer and fix msg_control if it has been used to point to the actual starting address of the buffer. This would be necessary to apply another feature to RTnet that allows to pass timestamps from the driver to userspace applications easily
(like SO_TIMESTAMP...). Please apply ;)

Manuel

Manuel Huber (1):
rtdm: Fix msghdr struct (cmsg) in sys_rtdm_recvmsg

 ksrc/skins/rtdm/syscall.c | 17 +++++++++++++----
 1 file changed, 13 insertions(+), 4 deletions(-)

1.8.3

From: Manuel Huber <[email protected]>

Whenever a new control message is put into msg_control buffer the actual address and the space left is saved to msg_control and msg_controllen. This allows adding messages as long as there is enough space left in the user-supplied buffer. Both fields have to be fixed again before passing them to the user by copying the original starting address of the buffer to msg_control and saving the actual amount of bytes written to the buffer to msg_controllen.

* Explicit use of __xn_put_user rather then __xn_copy_to_user
* Don't write back msg->msg_namelen

---
 ksrc/skins/rtdm/syscall.c | 17 +++++++++++++----
 1 file changed, 13 insertions(+), 4 deletions(-)

index 0ff5d40..7dd20e3 100644
@@ -79,22 +79,31 @@ static int sys_rtdm_recvmsg(struct pt_regs *regs)
 {
 	struct task_struct *p = current;
 	struct msghdr krnl_msg;
+	void *cmsg_control;
+	struct msghdr __user *usr_msg;
 	int ret;
 
-	if (unlikely(!access_wok(__xn_reg_arg2(regs),
+	usr_msg = (void __user *)__xn_reg_arg2(regs);
+
+	if (unlikely(!access_wok((void __user *)usr_msg,
 				 sizeof(krnl_msg)) ||
 		     __xn_copy_from_user(&krnl_msg,
-					 (void __user *)__xn_reg_arg2(regs),
+					 (void __user *)usr_msg,
 					 sizeof(krnl_msg))))
 		return -EFAULT;
 
+	cmsg_control = krnl_msg.msg_control;
+
 	ret = __rt_dev_recvmsg(p, __xn_reg_arg1(regs), &krnl_msg,
 			       __xn_reg_arg3(regs));
 	if (unlikely(ret < 0))
 		return ret;
 
-	if (unlikely(__xn_copy_to_user((void __user *)__xn_reg_arg2(regs),
-				       &krnl_msg, sizeof(krnl_msg))))
+	if (unlikely(__xn_put_user((typeof(krnl_msg.msg_controllen))(
+					   krnl_msg.msg_control - cmsg_control),
+				   (void __user *)&usr_msg->msg_controllen) ||
+		     __xn_put_user(krnl_msg.msg_flags,
+				   (void __user *)&(usr_msg->msg_flags))))
 		return -EFAULT;
 
 	return ret;

1.8.3

Oh, sorry; I forgot to add this....
Is this description okay:?

@param[in,out] msg Message descriptor as passed by the user, automatically mirrored to safe kernel memory in case of user mode call. If msg_control points to a valid buffer (not NULL) and msg_controllen is > 0, cmsghdr structures can be put into msg_control buffer by driver code. The msg_controllen field has to be decremented accordingly to mirror the actual space left and msg_control shall be incremented to always point to the next free byte.
If there is insufficient space left, MSG_CTRUNC will be set in msg_flags, but the call shall succeed.
Before returning, the actual number of bytes written to the msg_control buffer is written to msg_controllen and msg_control is reset to the
original
starting address.

Oh, sorry; I just assumed that msg_control has not been used, sorry... I checked the drivers and I believe it's only rtcan. There are two ways to fix rtcan:

1) I can break existing user code by using rt_put_cmsg from my RTnet patch (which does the same as put_cmsg from linux kernel) because that's how you are supposed to use msg_control buffer (to allow extra messages from every layer) and rtcan simply writes the timestamp to the buffer (no cmsghdr structure).

2) I can just fix current rtcan implementation to work with the proposed patch (fixing msg_controllen and msg_control according to the protocol in the driver) and not break existing user code but keep violating the original protocol (by not using cmsghr).

3) 1 + 2: Keep original interface to not break existing user code but add a warning when activating RTCAN_RTIOC_TAKE_TIMESTAMP (deprecated interface) and implement another interface that complies to the protocol and can be used instead (but adds overhead to code compared to the current interface...).

In case 1, 3 I would also like to move rt_put_cmsg to xenomai and maybe define SCM_TIMESTAMPNS64 in xenomai and use it in rtcan and RTnet to simply copy nanosecs_abs_t (by using rt_put_cmsg). As a legacy option I could implement SO_TIMESTAMPNS interface which converts nanosecs_abs_t to a timespec struct (as in my RTnet patch rt_nanosecs_to_ts ()).

I would add SCM_TIMESTAMPNS64 (we can rename that...) to rtdm header files as a standard option to get timestamps through recvmsg...

But I can't test rtcan (I think...); Is there someone who can? Change 2 is rather simple and should work (without testing) if I'm very careful. 1 and especially 3 are bigger changes (maybe 20-30 lines), this must be tested;

I personally like number 3 the most; I would start to implement number 2 send the patch and then start developing number 3.

What do you think?

Decrementing msg_controllen is not part of an interface between core and driver, is it? Let's keep it straight.

Who will set it, core layer or driver?

That's confusing. Better state that the core layer will update the user's msg_controllen field according to the number of bytes the driver reported to have written (via moving msg_control). This implies that the user's msg_control field is not updated.

Starting with option 2 sounds reasonable. We can then discuss introducing a generic timestamp feature that all RTDM protocol drivers can implement in the same way while keeping the existing ones of RTCAN and RTnet for backward compatibility.

Regarding CAN testing: You should be able to work with virtual CAN adapters, they should also produce timestamps. CC'ing Wolfgang to confirm this.

Jan

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 263 bytes
Desc: OpenPGP digital signature
URL: <http://www.xenomai.org/pipermail/xenomai/attachments/20130818/5e51b835/attachment.pgp>