[Intel-wired-lan] [PATCH net 01/13] ptp: Validate requests to enable time stamping of external signals.

[Intel-wired-lan] [PATCH net 01/13] ptp: Validate requests to enable time stamping of external signals.

[Richard Cochran]

Commit 415606588c61 ("PTP: introduce new versions of IOCTLs")
introduced a new external time stamp ioctl that validates the flags.
This patch extends the validation to ensure that at least one rising
or falling edge flag is set when enabling external time stamps.

Signed-off-by: Richard Cochran <richardcochran@gmail.com>
---
 drivers/ptp/ptp_chardev.c      | 18 +++++++++++++-----
 include/uapi/linux/ptp_clock.h |  1 +
 2 files changed, 14 insertions(+), 5 deletions(-)

diff --git a/drivers/ptp/ptp_chardev.c b/drivers/ptp/ptp_chardev.c
index 67d0199840fd..cbbe1237ff8d 100644
--- a/drivers/ptp/ptp_chardev.c
+++ b/drivers/ptp/ptp_chardev.c
@@ -149,11 +149,19 @@ long ptp_ioctl(struct posix_clock *pc, unsigned int cmd, unsigned long arg)
 			err = -EFAULT;
 			break;
 		}
-		if (((req.extts.flags & ~PTP_EXTTS_VALID_FLAGS) ||
-			req.extts.rsv[0] || req.extts.rsv[1]) &&
-			cmd == PTP_EXTTS_REQUEST2) {
-			err = -EINVAL;
-			break;
+		if (cmd == PTP_EXTTS_REQUEST2) {
+			/* Make sure no reserved bit is set. */
+			if ((req.extts.flags & ~PTP_EXTTS_VALID_FLAGS) ||
+			    req.extts.rsv[0] || req.extts.rsv[1]) {
+				err = -EINVAL;
+				break;
+			}
+			/* Ensure one of the rising/falling edge bits is set. */
+			if ((req.extts.flags & PTP_ENABLE_FEATURE) &&
+			    (req.extts.flags & PTP_EXTTS_EDGES) == 0) {
+				err = -EINVAL;
+				break;
+			}
 		} else if (cmd == PTP_EXTTS_REQUEST) {
 			req.extts.flags &= PTP_EXTTS_V1_VALID_FLAGS;
 			req.extts.rsv[0] = 0;
diff --git a/include/uapi/linux/ptp_clock.h b/include/uapi/linux/ptp_clock.h
index 59e89a1bc3bb..304059b1609d 100644
--- a/include/uapi/linux/ptp_clock.h
+++ b/include/uapi/linux/ptp_clock.h
@@ -31,6 +31,7 @@
 #define PTP_ENABLE_FEATURE (1<<0)
 #define PTP_RISING_EDGE    (1<<1)
 #define PTP_FALLING_EDGE   (1<<2)
+#define PTP_EXTTS_EDGES    (PTP_RISING_EDGE | PTP_FALLING_EDGE)
 
 /*
  * flag fields valid for the new PTP_EXTTS_REQUEST2 ioctl.
-- 
2.20.1

[PATCH net 01/13] ptp: Validate requests to enable time stamping of external signals.

[Richard Cochran]

Commit 415606588c61 ("PTP: introduce new versions of IOCTLs")
introduced a new external time stamp ioctl that validates the flags.
This patch extends the validation to ensure that at least one rising
or falling edge flag is set when enabling external time stamps.

Signed-off-by: Richard Cochran <richardcochran@gmail.com>
---
 drivers/ptp/ptp_chardev.c      | 18 +++++++++++++-----
 include/uapi/linux/ptp_clock.h |  1 +
 2 files changed, 14 insertions(+), 5 deletions(-)

diff --git a/drivers/ptp/ptp_chardev.c b/drivers/ptp/ptp_chardev.c
index 67d0199840fd..cbbe1237ff8d 100644
--- a/drivers/ptp/ptp_chardev.c
+++ b/drivers/ptp/ptp_chardev.c
@@ -149,11 +149,19 @@ long ptp_ioctl(struct posix_clock *pc, unsigned int cmd, unsigned long arg)
 			err = -EFAULT;
 			break;
 		}
-		if (((req.extts.flags & ~PTP_EXTTS_VALID_FLAGS) ||
-			req.extts.rsv[0] || req.extts.rsv[1]) &&
-			cmd == PTP_EXTTS_REQUEST2) {
-			err = -EINVAL;
-			break;
+		if (cmd == PTP_EXTTS_REQUEST2) {
+			/* Make sure no reserved bit is set. */
+			if ((req.extts.flags & ~PTP_EXTTS_VALID_FLAGS) ||
+			    req.extts.rsv[0] || req.extts.rsv[1]) {
+				err = -EINVAL;
+				break;
+			}
+			/* Ensure one of the rising/falling edge bits is set. */
+			if ((req.extts.flags & PTP_ENABLE_FEATURE) &&
+			    (req.extts.flags & PTP_EXTTS_EDGES) == 0) {
+				err = -EINVAL;
+				break;
+			}
 		} else if (cmd == PTP_EXTTS_REQUEST) {
 			req.extts.flags &= PTP_EXTTS_V1_VALID_FLAGS;
 			req.extts.rsv[0] = 0;
diff --git a/include/uapi/linux/ptp_clock.h b/include/uapi/linux/ptp_clock.h
index 59e89a1bc3bb..304059b1609d 100644
--- a/include/uapi/linux/ptp_clock.h
+++ b/include/uapi/linux/ptp_clock.h
@@ -31,6 +31,7 @@
 #define PTP_ENABLE_FEATURE (1<<0)
 #define PTP_RISING_EDGE    (1<<1)
 #define PTP_FALLING_EDGE   (1<<2)
+#define PTP_EXTTS_EDGES    (PTP_RISING_EDGE | PTP_FALLING_EDGE)
 
 /*
  * flag fields valid for the new PTP_EXTTS_REQUEST2 ioctl.
-- 
2.20.1

[Intel-wired-lan] [PATCH net 01/13] ptp: Validate requests to enable time stamping of external signals.

[Keller, Jacob E]

> -----Original Message-----
> From: Richard Cochran <richardcochran@gmail.com>
> Sent: Thursday, November 14, 2019 10:45 AM
> To: netdev at vger.kernel.org
> Cc: intel-wired-lan at lists.osuosl.org; David Miller <davem@davemloft.net>;
> Brandon Streiff <brandon.streiff@ni.com>; Hall, Christopher S
> <christopher.s.hall@intel.com>; Eugenia Emantayev <eugenia@mellanox.com>;
> Felipe Balbi <felipe.balbi@linux.intel.com>; Feras Daoud
> <ferasda@mellanox.com>; Keller, Jacob E <jacob.e.keller@intel.com>; Kirsher,
> Jeffrey T <jeffrey.t.kirsher@intel.com>; Sergei Shtylyov
> <sergei.shtylyov@cogentembedded.com>; Stefan Sorensen
> <stefan.sorensen@spectralink.com>
> Subject: [PATCH net 01/13] ptp: Validate requests to enable time stamping of
> external signals.
> 
> Commit 415606588c61 ("PTP: introduce new versions of IOCTLs")
> introduced a new external time stamp ioctl that validates the flags.
> This patch extends the validation to ensure that at least one rising
> or falling edge flag is set when enabling external time stamps.
> 
> Signed-off-by: Richard Cochran <richardcochran@gmail.com>
> ---
>  drivers/ptp/ptp_chardev.c      | 18 +++++++++++++-----
>  include/uapi/linux/ptp_clock.h |  1 +
>  2 files changed, 14 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/ptp/ptp_chardev.c b/drivers/ptp/ptp_chardev.c
> index 67d0199840fd..cbbe1237ff8d 100644
> --- a/drivers/ptp/ptp_chardev.c
> +++ b/drivers/ptp/ptp_chardev.c
> @@ -149,11 +149,19 @@ long ptp_ioctl(struct posix_clock *pc, unsigned int cmd,
> unsigned long arg)
>  			err = -EFAULT;
>  			break;
>  		}
> -		if (((req.extts.flags & ~PTP_EXTTS_VALID_FLAGS) ||
> -			req.extts.rsv[0] || req.extts.rsv[1]) &&
> -			cmd == PTP_EXTTS_REQUEST2) {
> -			err = -EINVAL;
> -			break;
> +		if (cmd == PTP_EXTTS_REQUEST2) {
> +			/* Make sure no reserved bit is set. */
> +			if ((req.extts.flags & ~PTP_EXTTS_VALID_FLAGS) ||
> +			    req.extts.rsv[0] || req.extts.rsv[1]) {
> +				err = -EINVAL;
> +				break;
> +			}
> +			/* Ensure one of the rising/falling edge bits is set. */
> +			if ((req.extts.flags & PTP_ENABLE_FEATURE) &&
> +			    (req.extts.flags & PTP_EXTTS_EDGES) == 0) {
> +				err = -EINVAL;
> +				break;
> +			}

Just to confirm, these new ioctls haven't been around long enough to be concerned about this change?

Thanks,
Jake

RE: [PATCH net 01/13] ptp: Validate requests to enable time stamping of external signals.

[Keller, Jacob E]

> -----Original Message-----
> From: Richard Cochran <richardcochran@gmail.com>
> Sent: Thursday, November 14, 2019 10:45 AM
> To: netdev@vger.kernel.org
> Cc: intel-wired-lan@lists.osuosl.org; David Miller <davem@davemloft.net>;
> Brandon Streiff <brandon.streiff@ni.com>; Hall, Christopher S
> <christopher.s.hall@intel.com>; Eugenia Emantayev <eugenia@mellanox.com>;
> Felipe Balbi <felipe.balbi@linux.intel.com>; Feras Daoud
> <ferasda@mellanox.com>; Keller, Jacob E <jacob.e.keller@intel.com>; Kirsher,
> Jeffrey T <jeffrey.t.kirsher@intel.com>; Sergei Shtylyov
> <sergei.shtylyov@cogentembedded.com>; Stefan Sorensen
> <stefan.sorensen@spectralink.com>
> Subject: [PATCH net 01/13] ptp: Validate requests to enable time stamping of
> external signals.
> 
> Commit 415606588c61 ("PTP: introduce new versions of IOCTLs")
> introduced a new external time stamp ioctl that validates the flags.
> This patch extends the validation to ensure that at least one rising
> or falling edge flag is set when enabling external time stamps.
> 
> Signed-off-by: Richard Cochran <richardcochran@gmail.com>
> ---
>  drivers/ptp/ptp_chardev.c      | 18 +++++++++++++-----
>  include/uapi/linux/ptp_clock.h |  1 +
>  2 files changed, 14 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/ptp/ptp_chardev.c b/drivers/ptp/ptp_chardev.c
> index 67d0199840fd..cbbe1237ff8d 100644
> --- a/drivers/ptp/ptp_chardev.c
> +++ b/drivers/ptp/ptp_chardev.c
> @@ -149,11 +149,19 @@ long ptp_ioctl(struct posix_clock *pc, unsigned int cmd,
> unsigned long arg)
>  			err = -EFAULT;
>  			break;
>  		}
> -		if (((req.extts.flags & ~PTP_EXTTS_VALID_FLAGS) ||
> -			req.extts.rsv[0] || req.extts.rsv[1]) &&
> -			cmd == PTP_EXTTS_REQUEST2) {
> -			err = -EINVAL;
> -			break;
> +		if (cmd == PTP_EXTTS_REQUEST2) {
> +			/* Make sure no reserved bit is set. */
> +			if ((req.extts.flags & ~PTP_EXTTS_VALID_FLAGS) ||
> +			    req.extts.rsv[0] || req.extts.rsv[1]) {
> +				err = -EINVAL;
> +				break;
> +			}
> +			/* Ensure one of the rising/falling edge bits is set. */
> +			if ((req.extts.flags & PTP_ENABLE_FEATURE) &&
> +			    (req.extts.flags & PTP_EXTTS_EDGES) == 0) {
> +				err = -EINVAL;
> +				break;
> +			}

Just to confirm, these new ioctls haven't been around long enough to be concerned about this change?

Thanks,
Jake

[Intel-wired-lan] [PATCH net 01/13] ptp: Validate requests to enable time stamping of external signals.

[Richard Cochran]

On Thu, Nov 14, 2019 at 07:06:58PM +0000, Keller, Jacob E wrote:
> Just to confirm, these new ioctls haven't been around long enough to be concerned about this change?

The "2" ioctls are about to appear in v5.4, and so I want to get the
flag checking in before the release if possible.

Thanks,
Richard

Re: [PATCH net 01/13] ptp: Validate requests to enable time stamping of external signals.

[Richard Cochran]

On Thu, Nov 14, 2019 at 07:06:58PM +0000, Keller, Jacob E wrote:
> Just to confirm, these new ioctls haven't been around long enough to be concerned about this change?

The "2" ioctls are about to appear in v5.4, and so I want to get the
flag checking in before the release if possible.

Thanks,
Richard

[Intel-wired-lan] [PATCH net 01/13] ptp: Validate requests to enable time stamping of external signals.

[Keller, Jacob E]

> -----Original Message-----
> From: netdev-owner at vger.kernel.org <netdev-owner@vger.kernel.org> On
> Behalf Of Richard Cochran
> Sent: Thursday, November 14, 2019 11:38 AM
> To: Keller, Jacob E <jacob.e.keller@intel.com>
> Cc: netdev at vger.kernel.org; intel-wired-lan at lists.osuosl.org; David Miller
> <davem@davemloft.net>; Brandon Streiff <brandon.streiff@ni.com>; Hall,
> Christopher S <christopher.s.hall@intel.com>; Eugenia Emantayev
> <eugenia@mellanox.com>; Felipe Balbi <felipe.balbi@linux.intel.com>; Feras
> Daoud <ferasda@mellanox.com>; Kirsher, Jeffrey T
> <jeffrey.t.kirsher@intel.com>; Sergei Shtylyov
> <sergei.shtylyov@cogentembedded.com>; Stefan Sorensen
> <stefan.sorensen@spectralink.com>
> Subject: Re: [PATCH net 01/13] ptp: Validate requests to enable time stamping of
> external signals.
> 
> On Thu, Nov 14, 2019 at 07:06:58PM +0000, Keller, Jacob E wrote:
> > Just to confirm, these new ioctls haven't been around long enough to be
> concerned about this change?
> 
> The "2" ioctls are about to appear in v5.4, and so I want to get the
> flag checking in before the release if possible.
> 
> Thanks,
> Richard

Yes agreed, we should land these fixes before 5.4 if we can, in order to ensure that the new ioctls always behaved the same way since their first major release introduction.

Regards,
Jake

RE: [PATCH net 01/13] ptp: Validate requests to enable time stamping of external signals.

[Keller, Jacob E]

> -----Original Message-----
> From: netdev-owner@vger.kernel.org <netdev-owner@vger.kernel.org> On
> Behalf Of Richard Cochran
> Sent: Thursday, November 14, 2019 11:38 AM
> To: Keller, Jacob E <jacob.e.keller@intel.com>
> Cc: netdev@vger.kernel.org; intel-wired-lan@lists.osuosl.org; David Miller
> <davem@davemloft.net>; Brandon Streiff <brandon.streiff@ni.com>; Hall,
> Christopher S <christopher.s.hall@intel.com>; Eugenia Emantayev
> <eugenia@mellanox.com>; Felipe Balbi <felipe.balbi@linux.intel.com>; Feras
> Daoud <ferasda@mellanox.com>; Kirsher, Jeffrey T
> <jeffrey.t.kirsher@intel.com>; Sergei Shtylyov
> <sergei.shtylyov@cogentembedded.com>; Stefan Sorensen
> <stefan.sorensen@spectralink.com>
> Subject: Re: [PATCH net 01/13] ptp: Validate requests to enable time stamping of
> external signals.
> 
> On Thu, Nov 14, 2019 at 07:06:58PM +0000, Keller, Jacob E wrote:
> > Just to confirm, these new ioctls haven't been around long enough to be
> concerned about this change?
> 
> The "2" ioctls are about to appear in v5.4, and so I want to get the
> flag checking in before the release if possible.
> 
> Thanks,
> Richard

Yes agreed, we should land these fixes before 5.4 if we can, in order to ensure that the new ioctls always behaved the same way since their first major release introduction.

Regards,
Jake