lib/vdso: Provide generic VDSO implementation
Hello Vincenzo Frascino,
This is a semi-automatic email about new static checker warnings.
The patch 00b26474c2f1: "lib/vdso: Provide generic VDSO implementation" from Jun 21, 2019, leads to the following Smatch complaint:
arch/x86/entry/vdso/vdso32/../../../../../lib/vdso/gettimeofday.c:120 __cvdso_clock_gettime32()
error: we previously assumed 'res' could be null (see line 107)
lib/vdso/gettimeofday.c
101 static __maybe_unused int
102 __cvdso_clock_gettime32(clockid_t clock, struct old_timespec32 *res)
103 {
104 struct __kernel_timespec ts;
105 int ret;
106
107 if (res = NULL)
^^^^^^^^^^^
108 goto fallback;
109
110 ret = __cvdso_clock_gettime(clock, &ts);
111
112 if (ret = 0) {
113 res->tv_sec = ts.tv_sec;
114 res->tv_nsec = ts.tv_nsec;
115 }
116
117 return ret;
118
119 fallback:
120 return clock_gettime_fallback(clock, (struct __kernel_timespec *)res);
^^^On x86 this "res" always gets dereferenced.
121 }
regards,
dan carpenter
See also __cvdso_clock_getres_time32().
lib/vdso/gettimeofday.c:226 __cvdso_clock_getres_time32() error: we previously assumed 'res' could be null (see line 213)
regards,
dan carpenter
Hi Dan,
thank you for testing my patches.
Quoted 34 lines · Dan Carpenter
Hello Vincenzo Frascino,
This is a semi-automatic email about new static checker warnings.
The patch
00b26474c2f1: "lib/vdso: Provide generic VDSO implementation" from Jun 21, 2019, leads to the following Smatch complaint:
arch/x86/entry/vdso/vdso32/../../../../../lib/vdso/gettimeofday.c:120__cvdso_clock_gettime32()
error: we previously assumed 'res' could be null (see line 107)
lib/vdso/gettimeofday.c101 static __maybe_unused int 102 __cvdso_clock_gettime32(clockid_t clock, struct old_timespec32 *res) 103 { 104 struct __kernel_timespec ts; 105 int ret; 106 107 if (res = NULL) ^^^^^^^^^^^ 108 goto fallback; 109 110 ret = __cvdso_clock_gettime(clock, &ts); 111 112 if (ret = 0) { 113 res->tv_sec = ts.tv_sec; 114 res->tv_nsec = ts.tv_nsec; 115 } 116 117 return ret; 118 119 fallback: 120 return clock_gettime_fallback(clock, (struct __kernel_timespec *)res); ^^^
Quoted 4 lines
On x86 this "res" always gets dereferenced.
121 }
I am not sure I understand the details of this bug report. As far as I can see "res" is never dereferenced in the vDSO library in this case, but it is passed to the system call unchanged.
static __always_inline
long clock_gettime_fallback(clockid_t _clkid, struct __kernel_timespec *_ts)
{
long ret;
asm ("syscall" : "=a" (ret), "=m" (*_ts) :
"0" (__NR_clock_gettime), "D" (_clkid), "S" (_ts) :
"rcx", "r11");
return ret;
}This is done to maintain consistency in between the returned error code of the syscall and of the vDSO library.
Could you please elaborate on why this bug has been reported?
Quoted 3 lines
regards,
dan carpenter
Signature · 2 lines
Regards, Vincenzo
Quoted 5 lines · Vincenzo Frascino
Quoted 5 lines
119 fallback: 120 return clock_gettime_fallback(clock, (struct __kernel_timespec *)res); ^^^On x86 this "res" always gets dereferenced.
Quoted 7 lines
Quoted 2 lines
121 }I am not sure I understand the details of this bug report. As far as I can see "res" is never dereferenced in the vDSO library in this case, but it is passed to the system call unchanged.
Oh yeah. Sorry. My bad. False positive. clock_gettime_fallback() doesn't necessarily dereference "res".
regards,
dan carpenter
Hi Dan,
Quoted 9 lines · Dan Carpenter
Quoted 8 lines · Vincenzo Frascino
Quoted 7 lines
119 fallback: 120 return clock_gettime_fallback(clock, (struct __kernel_timespec *)res); ^^^On x86 this "res" always gets dereferenced.
121 }
Quoted 8 lines
Quoted 4 lines
I am not sure I understand the details of this bug report. As far as I can see "res" is never dereferenced in the vDSO library in this case, but it is passed to the system call unchanged.
Oh yeah. Sorry. My bad. False positive.
clock_gettime_fallback()doesn't necessarily dereference "res".
No problem, better a false positive than an ignored negative :-)
Quoted 3 lines
regards,
dan carpenter
Signature · 2 lines
Regards, Vincenzo
Am 27.06.2019 11:09, schrieb Vincenzo Frascino:
Quoted 17 lines
Hi Dan,
Quoted 14 lines · Dan Carpenter
Quoted 12 lines · Vincenzo Frascino
Quoted 7 lines
119 fallback: 120 return clock_gettime_fallback(clock, (struct __kernel_timespec *)res); ^^^On x86 this "res" always gets dereferenced.
121 }I am not sure I understand the details of this bug report. As far as I can see "res" is never dereferenced in the vDSO library in this case, but it is passed to the system call unchanged.
Quoted 6 lines
Quoted 3 lines
Oh yeah. Sorry. My bad. False positive.
clock_gettime_fallback()doesn't necessarily dereference "res".No problem, better a false positive than an ignored negative :-)
maybe it would be clever to add a comment here indicating that this is intended and no problem ?
re,
wh
Quoted 24 lines · walter harms
Am 27.06.2019 11:09, schrieb Vincenzo Frascino:
Quoted 21 lines
Hi Dan,
Quoted 17 lines · Dan Carpenter
Quoted 5 lines · Vincenzo Frascino
Quoted 5 lines
119 fallback: 120 return clock_gettime_fallback(clock, (struct __kernel_timespec *)res); ^^^On x86 this "res" always gets dereferenced.
Quoted 7 lines
Quoted 2 lines
121 }I am not sure I understand the details of this bug report. As far as I can see "res" is never dereferenced in the vDSO library in this case, but it is passed to the system call unchanged.
Oh yeah. Sorry. My bad. False positive.
clock_gettime_fallback()doesn't necessarily dereference "res".
Quoted 6 lines
Quoted 2 lines
No problem, better a false positive than an ignored negative :-)
maybe it would be clever to add a comment here indicating that this is intended and no problem ?
No. The correct response is to fix Smatch. I will do it.
regards,
dan carpenter
Quoted 25 lines · Dan Carpenter
Quoted 24 lines · walter harms
Am 27.06.2019 11:09, schrieb Vincenzo Frascino:
Quoted 21 lines
Hi Dan,
Quoted 17 lines · Dan Carpenter
Quoted 5 lines · Vincenzo Frascino
Quoted 5 lines
119 fallback: 120 return clock_gettime_fallback(clock, (struct __kernel_timespec *)res); ^^^On x86 this "res" always gets dereferenced.
Quoted 7 lines
Quoted 2 lines
121 }I am not sure I understand the details of this bug report. As far as I can see "res" is never dereferenced in the vDSO library in this case, but it is passed to the system call unchanged.
Oh yeah. Sorry. My bad. False positive.
clock_gettime_fallback()doesn't necessarily dereference "res".
Quoted 9 lines
Quoted 6 lines
Quoted 2 lines
No problem, better a false positive than an ignored negative :-)
maybe it would be clever to add a comment here indicating that this is intended and no problem ?
No. The correct response is to fix Smatch. I will do it.
Agreed. Smatch should not report legitimate usages.
Quoted 3 lines
regards,
dan carpenter
Signature · 2 lines
Regards, Vincenzo