From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on inbox.vuxu.org X-Spam-Level: X-Spam-Status: No, score=-3.0 required=5.0 tests=HEADER_FROM_DIFFERENT_DOMAINS, MAILING_LIST_MULTI,RCVD_IN_DNSWL_MED,RCVD_IN_MSPIKE_H3, RCVD_IN_MSPIKE_WL autolearn=ham autolearn_force=no version=3.4.2 Received: from mother.openwall.net (mother.openwall.net [195.42.179.200]) by inbox.vuxu.org (OpenSMTPD) with SMTP id 803c8398 for ; Sun, 1 Mar 2020 09:14:09 +0000 (UTC) Received: (qmail 32077 invoked by uid 550); 1 Mar 2020 09:14:07 -0000 Mailing-List: contact musl-help@lists.openwall.com; run by ezmlm Precedence: bulk List-Post: List-Help: List-Unsubscribe: List-Subscribe: List-ID: Reply-To: musl@lists.openwall.com Received: (qmail 32059 invoked from network); 1 Mar 2020 09:14:06 -0000 Resent-From: Rich Felker Resent-Date: Sun, 1 Mar 2020 04:13:54 -0500 Resent-Message-ID: <20200301091354.GJ11469@brightrain.aerifal.cx> Resent-To: musl@lists.openwall.com Date: Sun, 1 Mar 2020 02:17:37 -0500 From: Rich Felker To: musl@lists.openwall.com Message-ID: <20200301071737.GI11469@brightrain.aerifal.cx> References: <20200301065730.146013-1-liujie1@huawei.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20200301065730.146013-1-liujie1@huawei.com> User-Agent: Mutt/1.5.21 (2010-09-15) Subject: Re: [musl] [PATCH] musl: lutimes: Add checks for input parameters 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 > --- > 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 > #include > #include > +#include > +#include > > 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. Rich