| Message ID | 20250312-jk-net-fixes-supported-extts-flags-v2-2-ea930ba82459@intel.com (mailing list archive) |
|---|---|
| State | Accepted |
| Delegated to: | Geert Uytterhoeven |
| Headers | show |
| Series | net: ptp: fix egregious supported flag checks | expand |
Hi Jacob, Thanks for your work. On 2025-03-12 15:15:51 -0700, Jacob Keller wrote: > The ravb_ptp_extts() function checks the flags coming from the > PTP_EXTTS_REQUEST ioctl, to ensure that future flags are not accepted on > accident. > > This was updated to 'honor' the PTP_STRICT_FLAGS in commit 6138e687c7b6 > ("ptp: Introduce strict checking of external time stamp options."). > However, the driver does not *actually* validate the flags. > > I originally fixed this driver to reject future flags in commit > 592025a03b34 ("renesas: reject unsupported external timestamp flags"). It > is still unclear whether this hardware timestamps the rising, falling, or > both edges of the input signal. > > Accepting requests with PTP_STRICT_FLAGS is a bug, as this could lead to > users mistakenly assuming a request with PTP_RISING_EDGE actually > timestamps the rising edge only. > > Reject requests with PTP_STRICT_FLAGS (and hence all PTP_EXTTS_REQUEST2 > requests) until someone with access to the datasheet or hardware knowledge > can confirm the timestamping behavior and update this driver. > > Fixes: 6138e687c7b6 ("ptp: Introduce strict checking of external time stamp options.") > Signed-off-by: Jacob Keller <jacob.e.keller@intel.com> Reviewed-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se> > --- > drivers/net/ethernet/renesas/ravb_ptp.c | 3 +-- > 1 file changed, 1 insertion(+), 2 deletions(-) > > diff --git a/drivers/net/ethernet/renesas/ravb_ptp.c b/drivers/net/ethernet/renesas/ravb_ptp.c > index 6e4ef7af27bf31ab2aad8e06a65e0ede6046e3c0..b4365906669f3bd40953813e263aeaafd2e1eb70 100644 > --- a/drivers/net/ethernet/renesas/ravb_ptp.c > +++ b/drivers/net/ethernet/renesas/ravb_ptp.c > @@ -179,8 +179,7 @@ static int ravb_ptp_extts(struct ptp_clock_info *ptp, > /* Reject requests with unsupported flags */ > if (req->flags & ~(PTP_ENABLE_FEATURE | > PTP_RISING_EDGE | > - PTP_FALLING_EDGE | > - PTP_STRICT_FLAGS)) > + PTP_FALLING_EDGE)) > return -EOPNOTSUPP; > > if (req->index) > > -- > 2.48.1.397.gec9d649cc640 >
On Wed, Mar 12, 2025 at 03:15:51PM -0700, Jacob Keller wrote: > The ravb_ptp_extts() function checks the flags coming from the > PTP_EXTTS_REQUEST ioctl, to ensure that future flags are not accepted on > accident. > > This was updated to 'honor' the PTP_STRICT_FLAGS in commit 6138e687c7b6 > ("ptp: Introduce strict checking of external time stamp options."). > However, the driver does not *actually* validate the flags. > > I originally fixed this driver to reject future flags in commit > 592025a03b34 ("renesas: reject unsupported external timestamp flags"). It > is still unclear whether this hardware timestamps the rising, falling, or > both edges of the input signal. > > Accepting requests with PTP_STRICT_FLAGS is a bug, as this could lead to > users mistakenly assuming a request with PTP_RISING_EDGE actually > timestamps the rising edge only. > > Reject requests with PTP_STRICT_FLAGS (and hence all PTP_EXTTS_REQUEST2 > requests) until someone with access to the datasheet or hardware knowledge > can confirm the timestamping behavior and update this driver. > > Fixes: 6138e687c7b6 ("ptp: Introduce strict checking of external time stamp options.") > Signed-off-by: Jacob Keller <jacob.e.keller@intel.com> Reviewed-by: Simon Horman <horms@kernel.org>
diff --git a/drivers/net/ethernet/renesas/ravb_ptp.c b/drivers/net/ethernet/renesas/ravb_ptp.c index 6e4ef7af27bf31ab2aad8e06a65e0ede6046e3c0..b4365906669f3bd40953813e263aeaafd2e1eb70 100644 --- a/drivers/net/ethernet/renesas/ravb_ptp.c +++ b/drivers/net/ethernet/renesas/ravb_ptp.c @@ -179,8 +179,7 @@ static int ravb_ptp_extts(struct ptp_clock_info *ptp, /* Reject requests with unsupported flags */ if (req->flags & ~(PTP_ENABLE_FEATURE | PTP_RISING_EDGE | - PTP_FALLING_EDGE | - PTP_STRICT_FLAGS)) + PTP_FALLING_EDGE)) return -EOPNOTSUPP; if (req->index)
The ravb_ptp_extts() function checks the flags coming from the PTP_EXTTS_REQUEST ioctl, to ensure that future flags are not accepted on accident. This was updated to 'honor' the PTP_STRICT_FLAGS in commit 6138e687c7b6 ("ptp: Introduce strict checking of external time stamp options."). However, the driver does not *actually* validate the flags. I originally fixed this driver to reject future flags in commit 592025a03b34 ("renesas: reject unsupported external timestamp flags"). It is still unclear whether this hardware timestamps the rising, falling, or both edges of the input signal. Accepting requests with PTP_STRICT_FLAGS is a bug, as this could lead to users mistakenly assuming a request with PTP_RISING_EDGE actually timestamps the rising edge only. Reject requests with PTP_STRICT_FLAGS (and hence all PTP_EXTTS_REQUEST2 requests) until someone with access to the datasheet or hardware knowledge can confirm the timestamping behavior and update this driver. Fixes: 6138e687c7b6 ("ptp: Introduce strict checking of external time stamp options.") Signed-off-by: Jacob Keller <jacob.e.keller@intel.com> --- drivers/net/ethernet/renesas/ravb_ptp.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-)