This is the mail archive of the libc-alpha@sourceware.org mailing list for the glibc project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH 1/2] y2038: linux: Provide __utimensat64 implementation


On Mon, 28 Oct 2019, Lukasz Majewski wrote:

> The new helper function - __utimensat64_helper - has been introduced to
> facilitate code re-usage on function providing futimens syscall handling.
> It also checks if passed nanoseconds are in the valid range as well as if one of
> two special values - UTIME_NOW and UTIME_OMIT were not passed.

In general we don't need such a check in userspace if the nanoseconds 
value will be passed to the kernel and it will do its own check.  I don't 
see such a check in the original code.  Is it being added to give the 
EINVAL error precedence over EOVERFLOW, in the case where a 64-bit time 
value with invalid nanoseconds is passed but a syscall using 32-bit time 
ends up being used?

> +  if (tsp64 && (! valid_nanoseconds(tsp64[0].tv_nsec)
> +                || ! valid_nanoseconds(tsp64[1].tv_nsec)))

Note missing space before '('.

> +    {
> +      if (tsp64[0].tv_nsec != UTIME_NOW && tsp64[0].tv_nsec != UTIME_OMIT
> +          && tsp64[1].tv_nsec != UTIME_NOW && tsp64[1].tv_nsec != UTIME_OMIT)
> +        {
> +          __set_errno (EINVAL);

I don't think this logic is correct; it would allow through cases where 
one timestamp is invalid and the other is UTIME_NOW or UTIME_OMIT, for 
example.

It might be better to have another helper function, e.g. 
valid_nanoseconds_for_utimensat, which calls valid_nanoseconds but also 
accepts the UTIME_* constants.

-- 
Joseph S. Myers
joseph@codesourcery.com


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]