| Message ID | 20200804234308.1303022-1-olteanv@gmail.com |
|---|---|
| State | Superseded |
| Delegated to: | David Miller |
| Headers | show |
| Series | [v2,net-next] ptp: only allow phase values lower than 1 period | expand |
On 8/4/2020 4:43 PM, Vladimir Oltean wrote: > The way we define the phase (the difference between the time of the > signal's rising edge, and the closest integer multiple of the period), > it doesn't make sense to have a phase value larger than 1 period. > > So deny these settings coming from the user. > > Signed-off-by: Vladimir Oltean <olteanv@gmail.com> > --- > Changes in v2: > Be sure to also deny the case where the period is equal to the phase. > This represents a 360 degree offset, which is equivalent to a phase of > zero, so it should be rejected on the grounds of having a modulo > equivalent as well. > > drivers/ptp/ptp_chardev.c | 13 +++++++++++++ > 1 file changed, 13 insertions(+) > > diff --git a/drivers/ptp/ptp_chardev.c b/drivers/ptp/ptp_chardev.c > index e0e6f85966e1..ee48eb61b49c 100644 > --- a/drivers/ptp/ptp_chardev.c > +++ b/drivers/ptp/ptp_chardev.c > @@ -218,6 +218,19 @@ long ptp_ioctl(struct posix_clock *pc, unsigned int cmd, unsigned long arg) > break; > } > } > + if (perout->flags & PTP_PEROUT_PHASE) { > + /* > + * The phase should be specified modulo the > + * period, therefore anything larger than 1 > + * period is invalid. > + */ A nit: this could read "therefore anything equal or larger than 1 period is invalid"? a number modulo itself is 0, right? and we use ">=" below as well now. I do think it's relatively clear from context so it probably isn't worth a re-roll. > + if (perout->phase.sec > perout->period.sec || > + (perout->phase.sec == perout->period.sec && > + perout->phase.nsec >= perout->period.nsec)) { > + err = -ERANGE; > + break; > + } > + } > } else if (cmd == PTP_PEROUT_REQUEST) { > req.perout.flags &= PTP_PEROUT_V1_VALID_FLAGS; > req.perout.rsv[0] = 0; >
On Tue, Aug 04, 2020 at 05:04:47PM -0700, Jacob Keller wrote: > > A nit: this could read "therefore anything equal or larger than 1 period > is invalid"? a number modulo itself is 0, right? and we use ">=" below > as well now. > Thanks, I've corrected that too, now. -Vladimir
diff --git a/drivers/ptp/ptp_chardev.c b/drivers/ptp/ptp_chardev.c index e0e6f85966e1..ee48eb61b49c 100644 --- a/drivers/ptp/ptp_chardev.c +++ b/drivers/ptp/ptp_chardev.c @@ -218,6 +218,19 @@ long ptp_ioctl(struct posix_clock *pc, unsigned int cmd, unsigned long arg) break; } } + if (perout->flags & PTP_PEROUT_PHASE) { + /* + * The phase should be specified modulo the + * period, therefore anything larger than 1 + * period is invalid. + */ + if (perout->phase.sec > perout->period.sec || + (perout->phase.sec == perout->period.sec && + perout->phase.nsec >= perout->period.nsec)) { + err = -ERANGE; + break; + } + } } else if (cmd == PTP_PEROUT_REQUEST) { req.perout.flags &= PTP_PEROUT_V1_VALID_FLAGS; req.perout.rsv[0] = 0;
The way we define the phase (the difference between the time of the signal's rising edge, and the closest integer multiple of the period), it doesn't make sense to have a phase value larger than 1 period. So deny these settings coming from the user. Signed-off-by: Vladimir Oltean <olteanv@gmail.com> --- Changes in v2: Be sure to also deny the case where the period is equal to the phase. This represents a 360 degree offset, which is equivalent to a phase of zero, so it should be rejected on the grounds of having a modulo equivalent as well. drivers/ptp/ptp_chardev.c | 13 +++++++++++++ 1 file changed, 13 insertions(+)