lkml.org 
[lkml]   [2019]   [Feb]   [25]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
From
Date
SubjectRe: [PATCH] time64: Avoid undefined behaviour in timespec64_add()
On Mon, Feb 25, 2019 at 10:01 AM Arnd Bergmann <arnd@arndb.de> wrote:
> On Mon, Feb 25, 2019 at 5:53 AM Deepa Dinamani <deepa.kernel@gmail.com> wrote:
> > On Sun, Feb 24, 2019 at 7:13 PM Hongbo Yao <yaohongbo@huawei.com> wrote:

> arch/arm/xen/enlighten.c: *ts = timespec64_add(now, ts_monotonic);
> arch/arm/xen/enlighten.c: system_time = timespec64_add(now,
> tk->wall_to_monotonic);
> drivers/net/ethernet/cadence/macb_ptp.c: now =
> timespec64_add(now, then);
> drivers/net/ethernet/intel/igb/igb_main.c: ts =
> timespec64_add(adapter->perout[0].start,
> drivers/net/ethernet/intel/igb/igb_main.c: ts =
> timespec64_add(adapter->perout[1].start,
> drivers/net/ethernet/intel/igb/igb_ptp.c: now = timespec64_add(now, then);
> fs/cifs/dfs_cache.c: return timespec64_add(now, ts);
> include/linux/rtc.h: *to_set = timespec64_add(*now, delay);
> include/linux/time64.h:static inline struct timespec64
> timespec64_add(struct timespec64 lhs,
> kernel/time/timekeeping.c: tmp = timespec64_add(tk_xtime(tk), *ts);
> kernel/time/timekeeping.c:
> timespec64_add(timekeeping_suspend_time, delta_delta);
> net/ceph/messenger.c: ts =
> timespec64_add(con->last_keepalive_ack, ts);
>
> It looks like an actual overflow would be really bad in most of these,
> regardless of the undefined behavior.

Side note: I was wondering whether some of those should use
timespec64_add_ns() instead of converting a 64-bit nanosecond
value to timespec64 first. My conclusion was "no, since
timespec64_add_ns() avoids a 64-bit division by assuming that
the nanoseconds are small", and in fact we probably need the oppose
and change two drivers the other way:

diff --git a/drivers/net/ethernet/cadence/macb_ptp.c
b/drivers/net/ethernet/cadence/macb_ptp.c
index a6dc47edc4cf..0d5ebde29c0d 100644
--- a/drivers/net/ethernet/cadence/macb_ptp.c
+++ b/drivers/net/ethernet/cadence/macb_ptp.c
@@ -160,7 +160,7 @@ static int gem_ptp_adjfine(struct ptp_clock_info
*ptp, long scaled_ppm)
static int gem_ptp_adjtime(struct ptp_clock_info *ptp, s64 delta)
{
struct macb *bp = container_of(ptp, struct macb, ptp_clock_info);
- struct timespec64 now, then = ns_to_timespec64(delta);
+ struct timespec64 now;
u32 adj, sign = 0;

if (delta < 0) {
@@ -170,7 +170,7 @@ static int gem_ptp_adjtime(struct ptp_clock_info
*ptp, s64 delta)

if (delta > TSU_NSEC_MAX_VAL) {
gem_tsu_get_time(&bp->ptp_clock_info, &now);
- now = timespec64_add(now, then);
+ now = timespec64_add(now, ns_to_timespec64(delta));

gem_tsu_set_time(&bp->ptp_clock_info,
(const struct timespec64 *)&now);
diff --git a/drivers/net/ethernet/intel/i40e/i40e_ptp.c
b/drivers/net/ethernet/intel/i40e/i40e_ptp.c
index 5fb4353c742b..4efcba0252a4 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_ptp.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_ptp.c
@@ -151,7 +151,7 @@ static int i40e_ptp_adjtime(struct ptp_clock_info
*ptp, s64 delta)
mutex_lock(&pf->tmreg_lock);

i40e_ptp_read(pf, &now, NULL);
- timespec64_add_ns(&now, delta);
+ timespec64_add_ns(&now, ns_to_timespec64(delta));
i40e_ptp_write(pf, (const struct timespec64 *)&now);

mutex_unlock(&pf->tmreg_lock);

Arnd

\
 
 \ /
  Last update: 2019-02-25 10:08    [W:0.054 / U:8.352 seconds]
©2003-2018 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site