On Mon, Jan 4, 2016 at 11:27 AM, Alison Schofield amsfield22@gmail.com wrote:
resending for review...
On Fri, Nov 20, 2015 at 10:23:17AM -0800, Alison Schofield wrote:
divamnt stores a start_time at init and uses it to calculate elapsed time. These operations are all internal to divamnt.
Address struct timeval overflow on 32-bit systems by replacing: struct timeval with struct timespec64; do_gettimeofday() with getnstimeofday64(); the calculations that yield a safe and normalized elapsed time with timespec64_sub() which provides same.
Signed-off-by: Alison Schofield amsfield22@gmail.com
drivers/isdn/hardware/eicon/divamnt.c | 35
+++++++++++------------------------
1 file changed, 11 insertions(+), 24 deletions(-)
diff --git a/drivers/isdn/hardware/eicon/divamnt.c
b/drivers/isdn/hardware/eicon/divamnt.c
index 48db08d..7ac057e 100644 --- a/drivers/isdn/hardware/eicon/divamnt.c +++ b/drivers/isdn/hardware/eicon/divamnt.c @@ -45,7 +45,7 @@ char *DRIVERRELEASE_MNT = "2.0";
static wait_queue_head_t msgwaitq; static unsigned long opened; -static struct timeval start_time; +static struct timespec64 start_time;
extern int mntfunc_init(int *, void **, unsigned long); extern void mntfunc_finit(void); @@ -88,28 +88,15 @@ int diva_os_copy_from_user(void *os_handle, void
*dst, const void __user *src,
*/ void diva_os_get_time(dword *sec, dword *usec) {
struct timeval tv;do_gettimeofday(&tv);if (tv.tv_sec > start_time.tv_sec) {if (start_time.tv_usec > tv.tv_usec) {tv.tv_sec--;tv.tv_usec += 1000000;}*sec = (dword) (tv.tv_sec - start_time.tv_sec);*usec = (dword) (tv.tv_usec - start_time.tv_usec);} else if (tv.tv_sec == start_time.tv_sec) {*sec = 0;if (start_time.tv_usec < tv.tv_usec) {*usec = (dword) (tv.tv_usec - start_time.tv_usec);} else {*usec = 0;}} else {*sec = (dword) tv.tv_sec;*usec = (dword) tv.tv_usec;}
timespec64_sub(lhs, rhs) assumes lhs > rhs always.
But, the original code doesn't seem to think this is always the case because of the else condition. The first 2 cases where start_time < curr_time, start_time == curr_time are handled in the change. But, think you have missed the case when start_time > curr_time?
struct timespec64 curr_time;struct timespec64 delta;getnstimeofday64(&curr_time);delta = timespec64_sub(curr_time, start_time);*sec = (dword) delta.tv_sec;*usec = (dword) (delta.tv_nsec / NSEC_PER_USEC);}
Here dword is defined as u32. Since this is only a difference in times, this might be warranted. But, the else part above might not fit in u32 because now you use 64 bit time. Maybe commit text should mention why this is still valid?
Since we are only concerned about difference monotonic times also should be okay.