| Message ID | 20170303160834.4125-2-mschmidt@redhat.com |
|---|---|
| State | Accepted, archived |
| Delegated to: | David Miller |
| Headers | show |
> It is possible to crash the kernel by accessing a PTP device while its > associated bnx2x interface is down. Before the interface is brought up, the > timecounter is not initialized, so accessing it results in NULL dereference. > > Fix it by checking if the interface is up. > > Use -ENETDOWN as the error code when the interface is down. > -EFAULT in bnx2x_ptp_adjfreq() did not seem right. > > Tested using phc_ctl get/set/adj/freq commands. > > Signed-off-by: Michal Schmidt <mschmidt@redhat.com> While I have no objections to the patch contents, does it even make sense to try adjusting frequencies on a DOWNed interface? Wouldn't it make more sense checking this in the calling context Instead?
Dne 5.3.2017 v 10:43 Mintz, Yuval napsal(a): >> It is possible to crash the kernel by accessing a PTP device while its >> associated bnx2x interface is down. Before the interface is brought up, the >> timecounter is not initialized, so accessing it results in NULL dereference. >> >> Fix it by checking if the interface is up. >> >> Use -ENETDOWN as the error code when the interface is down. >> -EFAULT in bnx2x_ptp_adjfreq() did not seem right. >> >> Tested using phc_ctl get/set/adj/freq commands. >> >> Signed-off-by: Michal Schmidt <mschmidt@redhat.com> > > While I have no objections to the patch contents, does it even make > sense to try adjusting frequencies on a DOWNed interface? > Wouldn't it make more sense checking this in the calling context > Instead? The caller does not know. A PTP device is not necessarily associated with a net device. Michal
diff --git a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c index d8d06fdfc4..d57290b9ea 100644 --- a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c +++ b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c @@ -13738,7 +13738,7 @@ static int bnx2x_ptp_adjfreq(struct ptp_clock_info *ptp, s32 ppb) if (!netif_running(bp->dev)) { DP(BNX2X_MSG_PTP, "PTP adjfreq called while the interface is down\n"); - return -EFAULT; + return -ENETDOWN; } if (ppb < 0) { @@ -13797,6 +13797,12 @@ static int bnx2x_ptp_adjtime(struct ptp_clock_info *ptp, s64 delta) { struct bnx2x *bp = container_of(ptp, struct bnx2x, ptp_clock_info); + if (!netif_running(bp->dev)) { + DP(BNX2X_MSG_PTP, + "PTP adjtime called while the interface is down\n"); + return -ENETDOWN; + } + DP(BNX2X_MSG_PTP, "PTP adjtime called, delta = %llx\n", delta); timecounter_adjtime(&bp->timecounter, delta); @@ -13809,6 +13815,12 @@ static int bnx2x_ptp_gettime(struct ptp_clock_info *ptp, struct timespec64 *ts) struct bnx2x *bp = container_of(ptp, struct bnx2x, ptp_clock_info); u64 ns; + if (!netif_running(bp->dev)) { + DP(BNX2X_MSG_PTP, + "PTP gettime called while the interface is down\n"); + return -ENETDOWN; + } + ns = timecounter_read(&bp->timecounter); DP(BNX2X_MSG_PTP, "PTP gettime called, ns = %llu\n", ns); @@ -13824,6 +13836,12 @@ static int bnx2x_ptp_settime(struct ptp_clock_info *ptp, struct bnx2x *bp = container_of(ptp, struct bnx2x, ptp_clock_info); u64 ns; + if (!netif_running(bp->dev)) { + DP(BNX2X_MSG_PTP, + "PTP settime called while the interface is down\n"); + return -ENETDOWN; + } + ns = timespec64_to_ns(ts); DP(BNX2X_MSG_PTP, "PTP settime called, ns = %llu\n", ns);
It is possible to crash the kernel by accessing a PTP device while its associated bnx2x interface is down. Before the interface is brought up, the timecounter is not initialized, so accessing it results in NULL dereference. Fix it by checking if the interface is up. Use -ENETDOWN as the error code when the interface is down. -EFAULT in bnx2x_ptp_adjfreq() did not seem right. Tested using phc_ctl get/set/adj/freq commands. Signed-off-by: Michal Schmidt <mschmidt@redhat.com> --- drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c | 20 +++++++++++++++++++- 1 file changed, 19 insertions(+), 1 deletion(-)