Commit e2a689ab authored by David S. Miller's avatar David S. Miller

Merge branch 'ptp-Validate-the-ancillary-ioctl-flags-more-carefully'

Richard Cochran says:

====================
ptp: Validate the ancillary ioctl flags more carefully.

The flags passed to the ioctls for periodic output signals and
time stamping of external signals were never checked, and thus formed
a useless ABI inadvertently.  More recently, a version 2 of the ioctls
was introduced in order make the flags meaningful.  This series
tightens up the checks on the new ioctl flags.

- Patch 1 ensures at least one edge flag is set for the new ioctl.
- Patches 2-7 are Jacob's recent checks, picking up the tags.
- Patch 8 introduces a "strict" flag for passing to the drivers when the
  new ioctl is used.
- Patches 9-12 implement the "strict" checking in the drivers.
- Patch 13 extends the test program to exercise combinations of flags.
====================
Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
parents 3df70afe 6eb54cbb
...@@ -273,6 +273,19 @@ static int mv88e6352_ptp_enable_extts(struct mv88e6xxx_chip *chip, ...@@ -273,6 +273,19 @@ static int mv88e6352_ptp_enable_extts(struct mv88e6xxx_chip *chip,
int pin; int pin;
int err; int err;
/* Reject requests with unsupported flags */
if (rq->extts.flags & ~(PTP_ENABLE_FEATURE |
PTP_RISING_EDGE |
PTP_FALLING_EDGE |
PTP_STRICT_FLAGS))
return -EOPNOTSUPP;
/* Reject requests to enable time stamping on both edges. */
if ((rq->extts.flags & PTP_STRICT_FLAGS) &&
(rq->extts.flags & PTP_ENABLE_FEATURE) &&
(rq->extts.flags & PTP_EXTTS_EDGES) == PTP_EXTTS_EDGES)
return -EOPNOTSUPP;
pin = ptp_find_pin(chip->ptp_clock, PTP_PF_EXTTS, rq->extts.index); pin = ptp_find_pin(chip->ptp_clock, PTP_PF_EXTTS, rq->extts.index);
if (pin < 0) if (pin < 0)
......
...@@ -6280,6 +6280,10 @@ static int tg3_ptp_enable(struct ptp_clock_info *ptp, ...@@ -6280,6 +6280,10 @@ static int tg3_ptp_enable(struct ptp_clock_info *ptp,
switch (rq->type) { switch (rq->type) {
case PTP_CLK_REQ_PEROUT: case PTP_CLK_REQ_PEROUT:
/* Reject requests with unsupported flags */
if (rq->perout.flags)
return -EOPNOTSUPP;
if (rq->perout.index != 0) if (rq->perout.index != 0)
return -EINVAL; return -EINVAL;
......
...@@ -521,6 +521,19 @@ static int igb_ptp_feature_enable_i210(struct ptp_clock_info *ptp, ...@@ -521,6 +521,19 @@ static int igb_ptp_feature_enable_i210(struct ptp_clock_info *ptp,
switch (rq->type) { switch (rq->type) {
case PTP_CLK_REQ_EXTTS: case PTP_CLK_REQ_EXTTS:
/* Reject requests with unsupported flags */
if (rq->extts.flags & ~(PTP_ENABLE_FEATURE |
PTP_RISING_EDGE |
PTP_FALLING_EDGE |
PTP_STRICT_FLAGS))
return -EOPNOTSUPP;
/* Reject requests failing to enable both edges. */
if ((rq->extts.flags & PTP_STRICT_FLAGS) &&
(rq->extts.flags & PTP_ENABLE_FEATURE) &&
(rq->extts.flags & PTP_EXTTS_EDGES) != PTP_EXTTS_EDGES)
return -EOPNOTSUPP;
if (on) { if (on) {
pin = ptp_find_pin(igb->ptp_clock, PTP_PF_EXTTS, pin = ptp_find_pin(igb->ptp_clock, PTP_PF_EXTTS,
rq->extts.index); rq->extts.index);
...@@ -551,6 +564,10 @@ static int igb_ptp_feature_enable_i210(struct ptp_clock_info *ptp, ...@@ -551,6 +564,10 @@ static int igb_ptp_feature_enable_i210(struct ptp_clock_info *ptp,
return 0; return 0;
case PTP_CLK_REQ_PEROUT: case PTP_CLK_REQ_PEROUT:
/* Reject requests with unsupported flags */
if (rq->perout.flags)
return -EOPNOTSUPP;
if (on) { if (on) {
pin = ptp_find_pin(igb->ptp_clock, PTP_PF_PEROUT, pin = ptp_find_pin(igb->ptp_clock, PTP_PF_PEROUT,
rq->perout.index); rq->perout.index);
......
...@@ -236,6 +236,19 @@ static int mlx5_extts_configure(struct ptp_clock_info *ptp, ...@@ -236,6 +236,19 @@ static int mlx5_extts_configure(struct ptp_clock_info *ptp,
if (!MLX5_PPS_CAP(mdev)) if (!MLX5_PPS_CAP(mdev))
return -EOPNOTSUPP; return -EOPNOTSUPP;
/* Reject requests with unsupported flags */
if (rq->extts.flags & ~(PTP_ENABLE_FEATURE |
PTP_RISING_EDGE |
PTP_FALLING_EDGE |
PTP_STRICT_FLAGS))
return -EOPNOTSUPP;
/* Reject requests to enable time stamping on both edges. */
if ((rq->extts.flags & PTP_STRICT_FLAGS) &&
(rq->extts.flags & PTP_ENABLE_FEATURE) &&
(rq->extts.flags & PTP_EXTTS_EDGES) == PTP_EXTTS_EDGES)
return -EOPNOTSUPP;
if (rq->extts.index >= clock->ptp_info.n_pins) if (rq->extts.index >= clock->ptp_info.n_pins)
return -EINVAL; return -EINVAL;
...@@ -290,6 +303,10 @@ static int mlx5_perout_configure(struct ptp_clock_info *ptp, ...@@ -290,6 +303,10 @@ static int mlx5_perout_configure(struct ptp_clock_info *ptp,
if (!MLX5_PPS_CAP(mdev)) if (!MLX5_PPS_CAP(mdev))
return -EOPNOTSUPP; return -EOPNOTSUPP;
/* Reject requests with unsupported flags */
if (rq->perout.flags)
return -EOPNOTSUPP;
if (rq->perout.index >= clock->ptp_info.n_pins) if (rq->perout.index >= clock->ptp_info.n_pins)
return -EINVAL; return -EINVAL;
......
...@@ -429,6 +429,10 @@ static int lan743x_ptp_perout(struct lan743x_adapter *adapter, int on, ...@@ -429,6 +429,10 @@ static int lan743x_ptp_perout(struct lan743x_adapter *adapter, int on,
int pulse_width = 0; int pulse_width = 0;
int perout_bit = 0; int perout_bit = 0;
/* Reject requests with unsupported flags */
if (perout->flags)
return -EOPNOTSUPP;
if (!on) { if (!on) {
lan743x_ptp_perout_off(adapter); lan743x_ptp_perout_off(adapter);
return 0; return 0;
......
...@@ -182,6 +182,13 @@ static int ravb_ptp_extts(struct ptp_clock_info *ptp, ...@@ -182,6 +182,13 @@ static int ravb_ptp_extts(struct ptp_clock_info *ptp,
struct net_device *ndev = priv->ndev; struct net_device *ndev = priv->ndev;
unsigned long flags; unsigned long flags;
/* Reject requests with unsupported flags */
if (req->flags & ~(PTP_ENABLE_FEATURE |
PTP_RISING_EDGE |
PTP_FALLING_EDGE |
PTP_STRICT_FLAGS))
return -EOPNOTSUPP;
if (req->index) if (req->index)
return -EINVAL; return -EINVAL;
...@@ -211,6 +218,10 @@ static int ravb_ptp_perout(struct ptp_clock_info *ptp, ...@@ -211,6 +218,10 @@ static int ravb_ptp_perout(struct ptp_clock_info *ptp,
unsigned long flags; unsigned long flags;
int error = 0; int error = 0;
/* Reject requests with unsupported flags */
if (req->flags)
return -EOPNOTSUPP;
if (req->index) if (req->index)
return -EINVAL; return -EINVAL;
......
...@@ -140,6 +140,10 @@ static int stmmac_enable(struct ptp_clock_info *ptp, ...@@ -140,6 +140,10 @@ static int stmmac_enable(struct ptp_clock_info *ptp,
switch (rq->type) { switch (rq->type) {
case PTP_CLK_REQ_PEROUT: case PTP_CLK_REQ_PEROUT:
/* Reject requests with unsupported flags */
if (rq->perout.flags)
return -EOPNOTSUPP;
cfg = &priv->pps[rq->perout.index]; cfg = &priv->pps[rq->perout.index];
cfg->start.tv_sec = rq->perout.start.sec; cfg->start.tv_sec = rq->perout.start.sec;
......
...@@ -469,6 +469,19 @@ static int ptp_dp83640_enable(struct ptp_clock_info *ptp, ...@@ -469,6 +469,19 @@ static int ptp_dp83640_enable(struct ptp_clock_info *ptp,
switch (rq->type) { switch (rq->type) {
case PTP_CLK_REQ_EXTTS: case PTP_CLK_REQ_EXTTS:
/* Reject requests with unsupported flags */
if (rq->extts.flags & ~(PTP_ENABLE_FEATURE |
PTP_RISING_EDGE |
PTP_FALLING_EDGE |
PTP_STRICT_FLAGS))
return -EOPNOTSUPP;
/* Reject requests to enable time stamping on both edges. */
if ((rq->extts.flags & PTP_STRICT_FLAGS) &&
(rq->extts.flags & PTP_ENABLE_FEATURE) &&
(rq->extts.flags & PTP_EXTTS_EDGES) == PTP_EXTTS_EDGES)
return -EOPNOTSUPP;
index = rq->extts.index; index = rq->extts.index;
if (index >= N_EXT_TS) if (index >= N_EXT_TS)
return -EINVAL; return -EINVAL;
...@@ -491,6 +504,9 @@ static int ptp_dp83640_enable(struct ptp_clock_info *ptp, ...@@ -491,6 +504,9 @@ static int ptp_dp83640_enable(struct ptp_clock_info *ptp,
return 0; return 0;
case PTP_CLK_REQ_PEROUT: case PTP_CLK_REQ_PEROUT:
/* Reject requests with unsupported flags */
if (rq->perout.flags)
return -EOPNOTSUPP;
if (rq->perout.index >= N_PER_OUT) if (rq->perout.index >= N_PER_OUT)
return -EINVAL; return -EINVAL;
return periodic_output(clock, rq, on, rq->perout.index); return periodic_output(clock, rq, on, rq->perout.index);
......
...@@ -149,11 +149,21 @@ long ptp_ioctl(struct posix_clock *pc, unsigned int cmd, unsigned long arg) ...@@ -149,11 +149,21 @@ long ptp_ioctl(struct posix_clock *pc, unsigned int cmd, unsigned long arg)
err = -EFAULT; err = -EFAULT;
break; break;
} }
if (((req.extts.flags & ~PTP_EXTTS_VALID_FLAGS) || if (cmd == PTP_EXTTS_REQUEST2) {
req.extts.rsv[0] || req.extts.rsv[1]) && /* Tell the drivers to check the flags carefully. */
cmd == PTP_EXTTS_REQUEST2) { req.extts.flags |= PTP_STRICT_FLAGS;
err = -EINVAL; /* Make sure no reserved bit is set. */
break; 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) { } else if (cmd == PTP_EXTTS_REQUEST) {
req.extts.flags &= PTP_EXTTS_V1_VALID_FLAGS; req.extts.flags &= PTP_EXTTS_V1_VALID_FLAGS;
req.extts.rsv[0] = 0; req.extts.rsv[0] = 0;
......
...@@ -31,13 +31,16 @@ ...@@ -31,13 +31,16 @@
#define PTP_ENABLE_FEATURE (1<<0) #define PTP_ENABLE_FEATURE (1<<0)
#define PTP_RISING_EDGE (1<<1) #define PTP_RISING_EDGE (1<<1)
#define PTP_FALLING_EDGE (1<<2) #define PTP_FALLING_EDGE (1<<2)
#define PTP_STRICT_FLAGS (1<<3)
#define PTP_EXTTS_EDGES (PTP_RISING_EDGE | PTP_FALLING_EDGE)
/* /*
* flag fields valid for the new PTP_EXTTS_REQUEST2 ioctl. * flag fields valid for the new PTP_EXTTS_REQUEST2 ioctl.
*/ */
#define PTP_EXTTS_VALID_FLAGS (PTP_ENABLE_FEATURE | \ #define PTP_EXTTS_VALID_FLAGS (PTP_ENABLE_FEATURE | \
PTP_RISING_EDGE | \ PTP_RISING_EDGE | \
PTP_FALLING_EDGE) PTP_FALLING_EDGE | \
PTP_STRICT_FLAGS)
/* /*
* flag fields valid for the original PTP_EXTTS_REQUEST ioctl. * flag fields valid for the original PTP_EXTTS_REQUEST ioctl.
......
...@@ -44,6 +44,46 @@ static int clock_adjtime(clockid_t id, struct timex *tx) ...@@ -44,6 +44,46 @@ static int clock_adjtime(clockid_t id, struct timex *tx)
} }
#endif #endif
static void show_flag_test(int rq_index, unsigned int flags, int err)
{
printf("PTP_EXTTS_REQUEST%c flags 0x%08x : (%d) %s\n",
rq_index ? '1' + rq_index : ' ',
flags, err, strerror(errno));
/* sigh, uClibc ... */
errno = 0;
}
static void do_flag_test(int fd, unsigned int index)
{
struct ptp_extts_request extts_request;
unsigned long request[2] = {
PTP_EXTTS_REQUEST,
PTP_EXTTS_REQUEST2,
};
unsigned int enable_flags[5] = {
PTP_ENABLE_FEATURE,
PTP_ENABLE_FEATURE | PTP_RISING_EDGE,
PTP_ENABLE_FEATURE | PTP_FALLING_EDGE,
PTP_ENABLE_FEATURE | PTP_RISING_EDGE | PTP_FALLING_EDGE,
PTP_ENABLE_FEATURE | (PTP_EXTTS_VALID_FLAGS + 1),
};
int err, i, j;
memset(&extts_request, 0, sizeof(extts_request));
extts_request.index = index;
for (i = 0; i < 2; i++) {
for (j = 0; j < 5; j++) {
extts_request.flags = enable_flags[j];
err = ioctl(fd, request[i], &extts_request);
show_flag_test(i, extts_request.flags, err);
extts_request.flags = 0;
err = ioctl(fd, request[i], &extts_request);
}
}
}
static clockid_t get_clockid(int fd) static clockid_t get_clockid(int fd)
{ {
#define CLOCKFD 3 #define CLOCKFD 3
...@@ -96,7 +136,8 @@ static void usage(char *progname) ...@@ -96,7 +136,8 @@ static void usage(char *progname)
" -s set the ptp clock time from the system time\n" " -s set the ptp clock time from the system time\n"
" -S set the system time from the ptp clock time\n" " -S set the system time from the ptp clock time\n"
" -t val shift the ptp clock time by 'val' seconds\n" " -t val shift the ptp clock time by 'val' seconds\n"
" -T val set the ptp clock time to 'val' seconds\n", " -T val set the ptp clock time to 'val' seconds\n"
" -z test combinations of rising/falling external time stamp flags\n",
progname); progname);
} }
...@@ -122,6 +163,7 @@ int main(int argc, char *argv[]) ...@@ -122,6 +163,7 @@ int main(int argc, char *argv[])
int adjtime = 0; int adjtime = 0;
int capabilities = 0; int capabilities = 0;
int extts = 0; int extts = 0;
int flagtest = 0;
int gettime = 0; int gettime = 0;
int index = 0; int index = 0;
int list_pins = 0; int list_pins = 0;
...@@ -138,7 +180,7 @@ int main(int argc, char *argv[]) ...@@ -138,7 +180,7 @@ int main(int argc, char *argv[])
progname = strrchr(argv[0], '/'); progname = strrchr(argv[0], '/');
progname = progname ? 1+progname : argv[0]; progname = progname ? 1+progname : argv[0];
while (EOF != (c = getopt(argc, argv, "cd:e:f:ghi:k:lL:p:P:sSt:T:v"))) { while (EOF != (c = getopt(argc, argv, "cd:e:f:ghi:k:lL:p:P:sSt:T:z"))) {
switch (c) { switch (c) {
case 'c': case 'c':
capabilities = 1; capabilities = 1;
...@@ -191,6 +233,9 @@ int main(int argc, char *argv[]) ...@@ -191,6 +233,9 @@ int main(int argc, char *argv[])
settime = 3; settime = 3;
seconds = atoi(optarg); seconds = atoi(optarg);
break; break;
case 'z':
flagtest = 1;
break;
case 'h': case 'h':
usage(progname); usage(progname);
return 0; return 0;
...@@ -322,6 +367,10 @@ int main(int argc, char *argv[]) ...@@ -322,6 +367,10 @@ int main(int argc, char *argv[])
} }
} }
if (flagtest) {
do_flag_test(fd, index);
}
if (list_pins) { if (list_pins) {
int n_pins = 0; int n_pins = 0;
if (ioctl(fd, PTP_CLOCK_GETCAPS, &caps)) { if (ioctl(fd, PTP_CLOCK_GETCAPS, &caps)) {
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment