[2/2] linux: clock_settime: Return proper value when passing NULL pointer
Commit Message
When in __clock_settime function (__TIMESIZE != 64) the const struct
timespec's *tp pointer is NULL, the Linux kernel syscall returns
-EFAULT.
Without this patch the glibc crashes (when dereferencing NULL pointer)
as the Linux kernel syscall is not reached at all.
There is no need for such check in the __clock_settime64, as this
pointer either goes directly to Linux kernel or the pointer to local
copy is used (ts64).
---
sysdeps/unix/sysv/linux/clock_settime.c | 9 ++++++++-
1 file changed, 8 insertions(+), 1 deletion(-)
Comments
On Fri, Nov 8, 2019 at 7:34 AM Lukasz Majewski <lukma@denx.de> wrote:
>
> When in __clock_settime function (__TIMESIZE != 64) the const struct
> timespec's *tp pointer is NULL, the Linux kernel syscall returns
> -EFAULT.
> Without this patch the glibc crashes (when dereferencing NULL pointer)
> as the Linux kernel syscall is not reached at all.
>
> There is no need for such check in the __clock_settime64, as this
> pointer either goes directly to Linux kernel or the pointer to local
> copy is used (ts64).
Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
Alistair
> ---
> sysdeps/unix/sysv/linux/clock_settime.c | 9 ++++++++-
> 1 file changed, 8 insertions(+), 1 deletion(-)
>
> diff --git a/sysdeps/unix/sysv/linux/clock_settime.c b/sysdeps/unix/sysv/linux/clock_settime.c
> index 6706dbb31f..e358a18998 100644
> --- a/sysdeps/unix/sysv/linux/clock_settime.c
> +++ b/sysdeps/unix/sysv/linux/clock_settime.c
> @@ -51,7 +51,14 @@ __clock_settime64 (clockid_t clock_id, const struct __timespec64 *tp)
> int
> __clock_settime (clockid_t clock_id, const struct timespec *tp)
> {
> - struct __timespec64 ts64 = valid_timespec_to_timespec64 (*tp);
> + struct __timespec64 ts64;
> +
> + if (tp == NULL)
> + {
> + __set_errno (EFAULT);
> + return -1;
> + }
> + ts64 = valid_timespec_to_timespec64 (*tp);
>
> return __clock_settime64 (clock_id, &ts64);
> }
> --
> 2.20.1
>
On Fri, 8 Nov 2019, Alistair Francis wrote:
> On Fri, Nov 8, 2019 at 7:34 AM Lukasz Majewski <lukma@denx.de> wrote:
> >
> > When in __clock_settime function (__TIMESIZE != 64) the const struct
> > timespec's *tp pointer is NULL, the Linux kernel syscall returns
> > -EFAULT.
> > Without this patch the glibc crashes (when dereferencing NULL pointer)
> > as the Linux kernel syscall is not reached at all.
> >
> > There is no need for such check in the __clock_settime64, as this
> > pointer either goes directly to Linux kernel or the pointer to local
> > copy is used (ts64).
>
> Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
This patch is contrary to glibc conventions. There is explicitly no
guarantee of whether a segfault or EFAULT occurs when a function is called
with invalid arguments. There should be no explicit checks for NULL
pointers in cases where a segfault will reliably occur otherwise and any
existing such checks should be removed from glibc.
https://sourceware.org/glibc/wiki/Style_and_Conventions#Invalid_pointers
(And note the POSIX specification of EFAULT, "The reliable detection of
this error cannot be guaranteed, and when not detected may result in the
generation of a signal, indicating an address violation, which is sent to
the process.".)
@@ -51,7 +51,14 @@ __clock_settime64 (clockid_t clock_id, const struct __timespec64 *tp)
int
__clock_settime (clockid_t clock_id, const struct timespec *tp)
{
- struct __timespec64 ts64 = valid_timespec_to_timespec64 (*tp);
+ struct __timespec64 ts64;
+
+ if (tp == NULL)
+ {
+ __set_errno (EFAULT);
+ return -1;
+ }
+ ts64 = valid_timespec_to_timespec64 (*tp);
return __clock_settime64 (clock_id, &ts64);
}