[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <b01082fe-e391-e80a-d31b-84651236b1e5@sholland.org>
Date: Sun, 1 Mar 2020 14:37:38 -0600
From: Samuel Holland <samuel@...lland.org>
To: musl@...ts.openwall.com, Rich Felker <dalias@...c.org>
Subject: Re: [PATCH] musl: lutimes: Add checks for input parameters
On 3/1/20 1:17 AM, Rich Felker wrote:
> On Sun, Mar 01, 2020 at 02:57:30PM +0800, Liu Jie wrote:
>> For the input parameter struct timeval tv, need to
>> determine whether it is invalid inputs.
>>
>> Signed-off-by: Liu Jie <liujie1@...wei.com>
>> ---
>> src/legacy/lutimes.c | 19 ++++++++++++++-----
>> 1 file changed, 14 insertions(+), 5 deletions(-)
>>
>> diff --git a/src/legacy/lutimes.c b/src/legacy/lutimes.c
>> index 2e5502d1..6e7e1be3 100644
>> --- a/src/legacy/lutimes.c
>> +++ b/src/legacy/lutimes.c
>> @@ -2,13 +2,22 @@
>> #include <sys/stat.h>
>> #include <sys/time.h>
>> #include <fcntl.h>
>> +#include <stdio.h>
>> +#include <errno.h>
>>
>> int lutimes(const char *filename, const struct timeval tv[2])
>> {
>> struct timespec times[2];
>> - times[0].tv_sec = tv[0].tv_sec;
>> - times[0].tv_nsec = tv[0].tv_usec * 1000;
>> - times[1].tv_sec = tv[1].tv_sec;
>> - times[1].tv_nsec = tv[1].tv_usec * 1000;
>> - return utimensat(AT_FDCWD, filename, times, AT_SYMLINK_NOFOLLOW);
>> + if (tv != NULL) {
>> + if (tv[0].tv_sec < 0 || tv[0].tv_usec < 0 ||
>> + tv[1].tv_sec < 0 || tv[1].tv_usec < 0) {
>> + errno = EINVAL;
>> + return -1;
>> + }
>> + times[0].tv_sec = tv[0].tv_sec;
>> + times[0].tv_nsec = tv[0].tv_usec * 1000;
>> + times[1].tv_sec = tv[1].tv_sec;
>> + times[1].tv_nsec = tv[1].tv_usec * 1000;
>> + }
>> + return utimensat(AT_FDCWD, filename, tv ? times : NULL, AT_SYMLINK_NOFOLLOW);
>> }
>> --
>> 2.17.1
>
> This patch causes uninitialized timespecs to be used if a null pointer
> is passed, silently corrupting data. If there is any historical
> documented precedent for this function accepting a null pointer and
> doing something meaningful, then the patch needs to do whatever that
> meaningful thing is rather than usign uninitialized data. If not, the
> preferred behavior is the current behavior: to crash so that the usage
> error is caught.
How do you see that uninitialized timespecs are used? times is only passed to
utimensat if tv is nonnull, and in that case times is initialized.
Regards,
Samuel
Powered by blists - more mailing lists
Confused about mailing lists and their use?
Read about mailing lists on Wikipedia
and check out these
guidelines on proper formatting of your messages.
