* Re: [bug report] lib/vdso: Provide generic VDSO implementation
2019-06-27 7:15 [bug report] lib/vdso: Provide generic VDSO implementation Dan Carpenter
@ 2019-06-27 7:18 ` Dan Carpenter
2019-06-27 8:58 ` Vincenzo Frascino
` (5 subsequent siblings)
6 siblings, 0 replies; 8+ messages in thread
From: Dan Carpenter @ 2019-06-27 7:18 UTC (permalink / raw)
To: kernel-janitors
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
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [bug report] lib/vdso: Provide generic VDSO implementation
2019-06-27 7:15 [bug report] lib/vdso: Provide generic VDSO implementation Dan Carpenter
2019-06-27 7:18 ` Dan Carpenter
@ 2019-06-27 8:58 ` Vincenzo Frascino
2019-06-27 9:07 ` Dan Carpenter
` (4 subsequent siblings)
6 siblings, 0 replies; 8+ messages in thread
From: Vincenzo Frascino @ 2019-06-27 8:58 UTC (permalink / raw)
To: kernel-janitors
Hi Dan,
thank you for testing my patches.
On 6/27/19 8:15 AM, Dan Carpenter wrote:
> 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 }
>
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?
> regards,
> dan carpenter
>
--
Regards,
Vincenzo
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [bug report] lib/vdso: Provide generic VDSO implementation
2019-06-27 7:15 [bug report] lib/vdso: Provide generic VDSO implementation Dan Carpenter
2019-06-27 7:18 ` Dan Carpenter
2019-06-27 8:58 ` Vincenzo Frascino
@ 2019-06-27 9:07 ` Dan Carpenter
2019-06-27 9:09 ` Vincenzo Frascino
` (3 subsequent siblings)
6 siblings, 0 replies; 8+ messages in thread
From: Dan Carpenter @ 2019-06-27 9:07 UTC (permalink / raw)
To: kernel-janitors
On Thu, Jun 27, 2019 at 09:58:18AM +0100, Vincenzo Frascino wrote:
> > 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.
>
Oh yeah. Sorry. My bad. False positive. clock_gettime_fallback()
doesn't necessarily dereference "res".
regards,
dan carpenter
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [bug report] lib/vdso: Provide generic VDSO implementation
2019-06-27 7:15 [bug report] lib/vdso: Provide generic VDSO implementation Dan Carpenter
` (2 preceding siblings ...)
2019-06-27 9:07 ` Dan Carpenter
@ 2019-06-27 9:09 ` Vincenzo Frascino
2019-06-27 10:26 ` walter harms
` (2 subsequent siblings)
6 siblings, 0 replies; 8+ messages in thread
From: Vincenzo Frascino @ 2019-06-27 9:09 UTC (permalink / raw)
To: kernel-janitors
Hi Dan,
On 6/27/19 10:07 AM, Dan Carpenter wrote:
> On Thu, Jun 27, 2019 at 09:58:18AM +0100, Vincenzo Frascino wrote:
>>> 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.
>>
>
> 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 :-)
> regards,
> dan carpenter
>
--
Regards,
Vincenzo
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [bug report] lib/vdso: Provide generic VDSO implementation
2019-06-27 7:15 [bug report] lib/vdso: Provide generic VDSO implementation Dan Carpenter
` (3 preceding siblings ...)
2019-06-27 9:09 ` Vincenzo Frascino
@ 2019-06-27 10:26 ` walter harms
2019-06-27 10:33 ` Dan Carpenter
2019-06-27 10:36 ` Vincenzo Frascino
6 siblings, 0 replies; 8+ messages in thread
From: walter harms @ 2019-06-27 10:26 UTC (permalink / raw)
To: kernel-janitors
Am 27.06.2019 11:09, schrieb Vincenzo Frascino:
> Hi Dan,
>
> On 6/27/19 10:07 AM, Dan Carpenter wrote:
>> On Thu, Jun 27, 2019 at 09:58:18AM +0100, Vincenzo Frascino wrote:
>>>> 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.
>>>
>>
>> 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
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [bug report] lib/vdso: Provide generic VDSO implementation
2019-06-27 7:15 [bug report] lib/vdso: Provide generic VDSO implementation Dan Carpenter
` (4 preceding siblings ...)
2019-06-27 10:26 ` walter harms
@ 2019-06-27 10:33 ` Dan Carpenter
2019-06-27 10:36 ` Vincenzo Frascino
6 siblings, 0 replies; 8+ messages in thread
From: Dan Carpenter @ 2019-06-27 10:33 UTC (permalink / raw)
To: kernel-janitors
On Thu, Jun 27, 2019 at 12:26:06PM +0200, walter harms wrote:
>
>
> Am 27.06.2019 11:09, schrieb Vincenzo Frascino:
> > Hi Dan,
> >
> > On 6/27/19 10:07 AM, Dan Carpenter wrote:
> >> On Thu, Jun 27, 2019 at 09:58:18AM +0100, Vincenzo Frascino wrote:
> >>>> 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.
> >>>
> >>
> >> 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 ?
>
No. The correct response is to fix Smatch. I will do it.
regards,
dan carpenter
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [bug report] lib/vdso: Provide generic VDSO implementation
2019-06-27 7:15 [bug report] lib/vdso: Provide generic VDSO implementation Dan Carpenter
` (5 preceding siblings ...)
2019-06-27 10:33 ` Dan Carpenter
@ 2019-06-27 10:36 ` Vincenzo Frascino
6 siblings, 0 replies; 8+ messages in thread
From: Vincenzo Frascino @ 2019-06-27 10:36 UTC (permalink / raw)
To: kernel-janitors
On 6/27/19 11:33 AM, Dan Carpenter wrote:
> On Thu, Jun 27, 2019 at 12:26:06PM +0200, walter harms wrote:
>>
>>
>> Am 27.06.2019 11:09, schrieb Vincenzo Frascino:
>>> Hi Dan,
>>>
>>> On 6/27/19 10:07 AM, Dan Carpenter wrote:
>>>> On Thu, Jun 27, 2019 at 09:58:18AM +0100, Vincenzo Frascino wrote:
>>>>>> 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.
>>>>>
>>>>
>>>> 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 ?
>>
>
> No. The correct response is to fix Smatch. I will do it.
>
Agreed. Smatch should not report legitimate usages.
> regards,
> dan carpenter
>
--
Regards,
Vincenzo
^ permalink raw reply [flat|nested] 8+ messages in thread