Re: [PATCH] LinuxPPS (with new syscalls API)

[Rodolfo Giometti]

On Wed, Jun 27, 2007 at 05:11:00PM +0100, David Woodhouse wrote:

> No, because you're passing a _kernel_ pointer to sys_time_pps_fetch()
> where it expects a userspace pointer. Use compat_alloc_user_space() to
> find somewhere to put it in user space, instead. Or change your internal
> __sys_time_pps_fetch() function to take a number of ticks instead of a
> pointer to a timespec, then call that directly with appropriate
> arguments, from both the normal and compat syscall routines.

Ok. Please see the attached patch.

Thanks a lot,

Rodolfo

-- 

GNU/Linux Solutions                  e-mail:    giometti@xxxxxxxxxxxx
Linux Device Driver                             giometti@xxxxxxxxx
Embedded Systems                     		giometti@xxxxxxxx
UNIX programming                     phone:     +39 349 2432127
diff --git a/drivers/pps/pps.c b/drivers/pps/pps.c
index befe292..820cc9a 100644
--- a/drivers/pps/pps.c
+++ b/drivers/pps/pps.c
@@ -26,6 +26,7 @@
 #include <linux/init.h>
 #include <linux/linkage.h>
 #include <linux/sched.h>
+#include <linux/compat.h>
 #include <linux/pps.h>
 #include <asm/uaccess.h>
 
@@ -284,13 +285,12 @@ sys_time_pps_getcap_exit:
 	return ret;
 }
 
-asmlinkage long sys_time_pps_fetch(int source, const int tsformat,
+static long __sys_time_pps_fetch(int source, const int tsformat,
 					struct pps_info __user *info, 
-					const struct timespec __user *timeout)
+					const struct timespec *timeout)
 {
 	unsigned long ticks;
 	struct pps_info pi;
-	struct timespec to;
 	int ret;
 
 	pps_dbg("%s: source %d", __FUNCTION__, source);
@@ -317,24 +317,19 @@ asmlinkage long sys_time_pps_fetch(int source, const int tsformat,
 	pps_source[source].go = 0;
 
  	/* Manage the timeout */
-	if (timeout) {
-		ret = copy_from_user(&to, timeout, sizeof(struct timespec));
-		if (ret)
-			goto sys_time_pps_fetch_exit;
-		if (to.tv_sec != -1) {
-			pps_dbg("timeout %ld.%09ld", to.tv_sec, to.tv_nsec);
-			ticks = to.tv_sec * HZ;
-			ticks += to.tv_nsec / (NSEC_PER_SEC / HZ);
-
-			if (ticks != 0) {
-				ret = wait_event_interruptible_timeout(
-					pps_source[source].queue,
-					pps_source[source].go, ticks);
-  				if (ret == 0) {
-					pps_dbg("timeout expired");
-					ret = -ETIMEDOUT;
-					goto sys_time_pps_fetch_exit;
-				}
+	if (timeout->tv_sec != -1) {
+		pps_dbg("timeout %ld.%09ld", timeout->tv_sec, timeout->tv_nsec);
+		ticks = timeout->tv_sec * HZ;
+		ticks += timeout->tv_nsec / (NSEC_PER_SEC / HZ);
+
+		if (ticks != 0) {
+			ret = wait_event_interruptible_timeout(
+				pps_source[source].queue,
+				pps_source[source].go, ticks);
+  			if (ret == 0) {
+				pps_dbg("timeout expired");
+				ret = -ETIMEDOUT;
+				goto sys_time_pps_fetch_exit;
 			}
 		}
  	} else
@@ -362,6 +357,44 @@ sys_time_pps_fetch_exit:
 	return ret;
 }
 
+asmlinkage long sys_time_pps_fetch(int source, const int tsformat,
+					struct pps_info __user *info, 
+					const struct timespec __user *timeout)
+{
+	int ret;
+	struct timespec to;
+
+	if (timeout) {
+		ret = copy_from_user(&to, timeout, sizeof(struct timespec));
+		if (ret)
+			return ret;
+	}
+	else
+		to.tv_sec = -1;	
+
+	return __sys_time_pps_fetch(source, tsformat, info, timeout);
+}
+
+#ifdef CONFIG_COMPAT
+asmlinkage long compat_sys_time_pps_fetch(int source, const int tsformat,
+				struct pps_info __user *info, 
+				const struct compat_timespec __user *timeout)
+{
+	int ret;
+	struct timespec to;
+
+	if (timeout) {
+		ret = get_compat_timespec(&to, timeout);
+		if (ret)
+			return -EFAULT;
+	}
+	else
+		to.tv_sec = -1;	
+	
+	return __sys_time_pps_fetch(source, tsformat, info, &to);
+}
+#endif
+
 /*
  * Module staff
  */