[PATCH 0/2] pps: add ioctl_compat support

[PATCH 0/2] pps: add ioctl_compat support

[Matt Ranostay]

This series fixes issues with 32-bit userspace + 64-bit kernel data
structures, and along with fixing incorrect ioctl definitions in
a ABI stable way

Matt Ranostay (2):
  pps: add ioctl_compat function to correct ioctl definitions
  pps: fix padding issue with PPS_FETCH for ioctl_compat

 drivers/pps/pps.c        | 123 +++++++++++++++++++++++++++++++++++------------
 include/uapi/linux/pps.h |  19 ++++++++
 2 files changed, 111 insertions(+), 31 deletions(-)

-- 
2.10.2

[PATCH 1/2] pps: add ioctl_compat function to correct ioctl definitions

[Matt Ranostay]

ioctl definitions use the pointer size of the architecture which
is fine when userspace and kernel are the same bitsize. This
patchset workarounds an issue with mixed bitsize kernel + userspace
by rewriting the cmd to the kernelspace architecture pointer size.

Cc: Rodolfo Giometti <giometti@enneenne.com>
Cc: Moritz Fischer <mdf@kernel.org>
Cc: George McCollister <george.mccollister@gmail.com>
Signed-off-by: Matt Ranostay <matt.ranostay@konsulko.com>
---
 drivers/pps/pps.c | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/drivers/pps/pps.c b/drivers/pps/pps.c
index 2f07cd615665..452ead5a5e52 100644
--- a/drivers/pps/pps.c
+++ b/drivers/pps/pps.c
@@ -242,6 +242,18 @@ static long pps_cdev_ioctl(struct file *file,
 	return 0;
 }
 
+#ifdef CONFIG_COMPAT
+static long pps_cdev_compat_ioctl(struct file *file,
+		unsigned int cmd, unsigned long arg)
+{
+	cmd = _IOC(_IOC_DIR(cmd), _IOC_TYPE(cmd), _IOC_NR(cmd), sizeof(void *));
+
+	return pps_cdev_ioctl(file, cmd, arg);
+}
+#else
+#define pps_cdev_compat_ioctl	NULL
+#endif
+
 static int pps_cdev_open(struct inode *inode, struct file *file)
 {
 	struct pps_device *pps = container_of(inode->i_cdev,
@@ -268,6 +280,7 @@ static const struct file_operations pps_cdev_fops = {
 	.llseek		= no_llseek,
 	.poll		= pps_cdev_poll,
 	.fasync		= pps_cdev_fasync,
+	.compat_ioctl	= pps_cdev_compat_ioctl,
 	.unlocked_ioctl	= pps_cdev_ioctl,
 	.open		= pps_cdev_open,
 	.release	= pps_cdev_release,
-- 
2.10.2

[PATCH 2/2] pps: fix padding issue with PPS_FETCH for ioctl_compat

[Matt Ranostay]

Issue is that x86 32-bit aligns to 4-bytes instead of 8-bytes
so this patchset works around the issue and corrects the data
returned in pps_fdata_compat.

Cc: Rodolfo Giometti <giometti@enneenne.com>
Cc: Moritz Fischer <mdf@kernel.org>
Cc: George McCollister <george.mccollister@gmail.com>
Signed-off-by: Matt Ranostay <matt.ranostay@konsulko.com>
---
 drivers/pps/pps.c        | 110 ++++++++++++++++++++++++++++++++++-------------
 include/uapi/linux/pps.h |  19 ++++++++
 2 files changed, 98 insertions(+), 31 deletions(-)

diff --git a/drivers/pps/pps.c b/drivers/pps/pps.c
index 452ead5a5e52..6eb0db37dd88 100644
--- a/drivers/pps/pps.c
+++ b/drivers/pps/pps.c
@@ -64,6 +64,43 @@ static int pps_cdev_fasync(int fd, struct file *file, int on)
 	return fasync_helper(fd, file, on, &pps->async_queue);
 }
 
+static int pps_cdev_pps_fetch(struct pps_device *pps, struct pps_fdata *fdata)
+{
+	unsigned int ev = pps->last_ev;
+	int err = 0;
+
+	/* Manage the timeout */
+	if (fdata->timeout.flags & PPS_TIME_INVALID)
+		err = wait_event_interruptible(pps->queue,
+				ev != pps->last_ev);
+	else {
+		unsigned long ticks;
+
+		dev_dbg(pps->dev, "timeout %lld.%09d\n",
+				(long long) fdata->timeout.sec,
+				fdata->timeout.nsec);
+		ticks = fdata->timeout.sec * HZ;
+		ticks += fdata->timeout.nsec / (NSEC_PER_SEC / HZ);
+
+		if (ticks != 0) {
+			err = wait_event_interruptible_timeout(
+					pps->queue,
+					ev != pps->last_ev,
+					ticks);
+			if (err == 0)
+				return -ETIMEDOUT;
+		}
+	}
+
+	/* Check for pending signals */
+	if (err == -ERESTARTSYS) {
+		dev_dbg(pps->dev, "pending signal caught\n");
+		return -EINTR;
+	}
+
+	return 0;
+}
+
 static long pps_cdev_ioctl(struct file *file,
 		unsigned int cmd, unsigned long arg)
 {
@@ -144,7 +181,6 @@ static long pps_cdev_ioctl(struct file *file,
 
 	case PPS_FETCH: {
 		struct pps_fdata fdata;
-		unsigned int ev;
 
 		dev_dbg(pps->dev, "PPS_FETCH\n");
 
@@ -152,36 +188,9 @@ static long pps_cdev_ioctl(struct file *file,
 		if (err)
 			return -EFAULT;
 
-		ev = pps->last_ev;
-
-		/* Manage the timeout */
-		if (fdata.timeout.flags & PPS_TIME_INVALID)
-			err = wait_event_interruptible(pps->queue,
-					ev != pps->last_ev);
-		else {
-			unsigned long ticks;
-
-			dev_dbg(pps->dev, "timeout %lld.%09d\n",
-					(long long) fdata.timeout.sec,
-					fdata.timeout.nsec);
-			ticks = fdata.timeout.sec * HZ;
-			ticks += fdata.timeout.nsec / (NSEC_PER_SEC / HZ);
-
-			if (ticks != 0) {
-				err = wait_event_interruptible_timeout(
-						pps->queue,
-						ev != pps->last_ev,
-						ticks);
-				if (err == 0)
-					return -ETIMEDOUT;
-			}
-		}
-
-		/* Check for pending signals */
-		if (err == -ERESTARTSYS) {
-			dev_dbg(pps->dev, "pending signal caught\n");
-			return -EINTR;
-		}
+		err = pps_cdev_pps_fetch(pps, &fdata);
+		if (err)
+			return err;
 
 		/* Return the fetched timestamp */
 		spin_lock_irq(&pps->lock);
@@ -246,8 +255,47 @@ static long pps_cdev_ioctl(struct file *file,
 static long pps_cdev_compat_ioctl(struct file *file,
 		unsigned int cmd, unsigned long arg)
 {
+	struct pps_device *pps = file->private_data;
+	void __user *uarg = (void __user *) arg;
+
 	cmd = _IOC(_IOC_DIR(cmd), _IOC_TYPE(cmd), _IOC_NR(cmd), sizeof(void *));
 
+	if (cmd == PPS_FETCH) {
+		struct pps_fdata_compat compat;
+		struct pps_fdata fdata;
+		int err;
+
+		dev_dbg(pps->dev, "PPS_FETCH\n");
+
+		err = copy_from_user(&compat, uarg, sizeof(struct pps_fdata_compat));
+		if (err)
+			return -EFAULT;
+
+		memcpy(&fdata.timeout, &compat.timeout,
+					sizeof(struct pps_ktime_compat));
+
+		err = pps_cdev_pps_fetch(pps, &fdata);
+		if (err)
+			return err;
+
+		/* Return the fetched timestamp */
+		spin_lock_irq(&pps->lock);
+
+		compat.info.assert_sequence = pps->assert_sequence;
+		compat.info.clear_sequence = pps->clear_sequence;
+		compat.info.current_mode = pps->current_mode;
+
+		memcpy(&compat.info.assert_tu, &pps->assert_tu,
+				sizeof(struct pps_ktime_compat));
+		memcpy(&compat.info.clear_tu, &pps->clear_tu,
+				sizeof(struct pps_ktime_compat));
+
+		spin_unlock_irq(&pps->lock);
+
+		return copy_to_user(uarg, &compat,
+				sizeof(struct pps_fdata_compat)) ? -EFAULT : 0;
+	}
+
 	return pps_cdev_ioctl(file, cmd, arg);
 }
 #else
diff --git a/include/uapi/linux/pps.h b/include/uapi/linux/pps.h
index a9bb1d93451a..c1cb3825a8bc 100644
--- a/include/uapi/linux/pps.h
+++ b/include/uapi/linux/pps.h
@@ -55,6 +55,12 @@ struct pps_ktime {
 	__s32 nsec;
 	__u32 flags;
 };
+
+struct pps_ktime_compat {
+	__s64 sec;
+	__s32 nsec;
+	__u32 flags;
+} __attribute__((packed, aligned(4)));
 #define PPS_TIME_INVALID	(1<<0)	/* used to specify timeout==NULL */
 
 struct pps_kinfo {
@@ -65,6 +71,14 @@ struct pps_kinfo {
 	int current_mode;		/* current mode bits */
 };
 
+struct pps_kinfo_compat {
+	__u32 assert_sequence;			/* seq. num. of assert event */
+	__u32 clear_sequence;			/* seq. num. of clear event */
+	struct pps_ktime_compat assert_tu;	/* time of assert event */
+	struct pps_ktime_compat clear_tu;	/* time of clear event */
+	int current_mode;			/* current mode bits */
+};
+
 struct pps_kparams {
 	int api_version;		/* API version # */
 	int mode;			/* mode bits */
@@ -114,6 +128,11 @@ struct pps_fdata {
 	struct pps_ktime timeout;
 };
 
+struct pps_fdata_compat {
+	struct pps_kinfo_compat info;
+	struct pps_ktime_compat timeout;
+};
+
 struct pps_bind_args {
 	int tsformat;	/* format of time stamps */
 	int edge;	/* selected event type */
-- 
2.10.2

Re: [PATCH 1/2] pps: add ioctl_compat function to correct ioctl definitions

[Rodolfo Giometti]

On 02/24/17 21:23, Matt Ranostay wrote:
> ioctl definitions use the pointer size of the architecture which
> is fine when userspace and kernel are the same bitsize. This
> patchset workarounds an issue with mixed bitsize kernel + userspace
> by rewriting the cmd to the kernelspace architecture pointer size.
>
> Cc: Rodolfo Giometti <giometti@enneenne.com>
> Cc: Moritz Fischer <mdf@kernel.org>
> Cc: George McCollister <george.mccollister@gmail.com>
> Signed-off-by: Matt Ranostay <matt.ranostay@konsulko.com>
> ---
>  drivers/pps/pps.c | 13 +++++++++++++
>  1 file changed, 13 insertions(+)

Acked-by: Rodolfo Giometti <giometti@enneenne.com>

-- 

HCE Engineering                      e-mail: giometti@hce-engineering.com
GNU/Linux Solutions                          giometti@enneenne.com
Linux Device Driver                          giometti@linux.it
Embedded Systems                     phone:  +39 349 2432127
UNIX programming                     skype:  rodolfo.giometti
Cosino Project - the quick prototyping embedded system - www.cosino.io
Freelance ICT Italia - Consulente ICT Italia - www.consulenti-ict.it

Re: [PATCH 2/2] pps: fix padding issue with PPS_FETCH for ioctl_compat

[Rodolfo Giometti]

On 02/24/17 21:23, Matt Ranostay wrote:
> Issue is that x86 32-bit aligns to 4-bytes instead of 8-bytes
> so this patchset works around the issue and corrects the data
> returned in pps_fdata_compat.
>
> Cc: Rodolfo Giometti <giometti@enneenne.com>
> Cc: Moritz Fischer <mdf@kernel.org>
> Cc: George McCollister <george.mccollister@gmail.com>
> Signed-off-by: Matt Ranostay <matt.ranostay@konsulko.com>
> ---
>  drivers/pps/pps.c        | 110 ++++++++++++++++++++++++++++++++++-------------
>  include/uapi/linux/pps.h |  19 ++++++++
>  2 files changed, 98 insertions(+), 31 deletions(-)

Acked-by: Rodolfo Giometti <giometti@enneenne.com>

-- 

HCE Engineering                      e-mail: giometti@hce-engineering.com
GNU/Linux Solutions                          giometti@enneenne.com
Linux Device Driver                          giometti@linux.it
Embedded Systems                     phone:  +39 349 2432127
UNIX programming                     skype:  rodolfo.giometti
Cosino Project - the quick prototyping embedded system - www.cosino.io
Freelance ICT Italia - Consulente ICT Italia - www.consulenti-ict.it