This is the mail archive of the
libc-alpha@sourceware.org
mailing list for the glibc project.
Re: [PATCH 1/2] y2038: linux: Provide __utimensat64 implementation
- From: Joseph Myers <joseph at codesourcery dot com>
- To: Lukasz Majewski <lukma at denx dot de>
- Cc: Paul Eggert <eggert at cs dot ucla dot edu>, Alistair Francis <alistair23 at gmail dot com>, Alistair Francis <alistair dot francis at wdc dot com>, GNU C Library <libc-alpha at sourceware dot org>, Adhemerval Zanella <adhemerval dot zanella at linaro dot org>, Florian Weimer <fweimer at redhat dot com>, Florian Weimer <fw at deneb dot enyo dot de>, Zack Weinberg <zackw at panix dot com>
- Date: Mon, 28 Oct 2019 18:11:41 +0000
- Subject: Re: [PATCH 1/2] y2038: linux: Provide __utimensat64 implementation
- Ironport-sdr: fFEqX4qZiaDR4E6ZBBsrbWR9uihoiSmp+m+klg1SSqMVi7oirPBMD0dT28XsGhHXzsuL2A+f6a m+3I2comNnqpIjEXhuK5mGOu+qPhDLGQnzutQ3zoUgOpzQZJd2ZxTAcLXw+cyW40JPYLltonkA ibi+svSAU/98jeqSuerQSAOv3kbxUupdcda99Ser+YeFCytvHTqR2I06vdAq2RH+40IVzDDcU1 As/p5ySXkmcd2t+0XITt9I1EjZty1afToudWB7wsAEthXmM2YXuvv2KtoFYGo70x7/Jwz0u4Gn wg8=
- Ironport-sdr: kRqfZAmEiTGPsI/YPPYTR38znKLFm/k6vFXvuiRg9nmIAlw9irQ5sfC7UvC5yKyxnfzhY9BhlW eQpv6G/LVz/pZl7v2fVZzce7EXYkqJy03YGdw828kn5YH8VzS7shNK+5sUwDNFYWzaCh7m2hBg 16zSrJl56ar1TniBLoNUdiEAdkOfAZja4fnvwo2od/+4S0GmviNn/bPWKm6jPkSFhS8OQpWnWD 8lXMrRtONv9v15KYVQzooJ4kqkK0R49k3EooTL08Y31ilqTajHvSD6CEO3V8Dwagh5Cub33ADC EvY=
- References: <20191028165926.10351-1-lukma@denx.de>
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