From mboxrd@z Thu Jan 1 00:00:00 1970 X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) on inbox.vuxu.org X-Spam-Level: X-Spam-Status: No, score=-3.4 required=5.0 tests=DKIM_SIGNED,DKIM_VALID, DKIM_VALID_AU,MAILING_LIST_MULTI,RCVD_IN_DNSWL_MED,RCVD_IN_MSPIKE_H3, RCVD_IN_MSPIKE_WL autolearn=ham autolearn_force=no version=3.4.4 Received: (qmail 15234 invoked from network); 12 Nov 2020 19:52:25 -0000 Received: from mother.openwall.net (195.42.179.200) by inbox.vuxu.org with ESMTPUTF8; 12 Nov 2020 19:52:25 -0000 Received: (qmail 20381 invoked by uid 550); 12 Nov 2020 19:52:23 -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 20359 invoked from network); 12 Nov 2020 19:52:22 -0000 X-Virus-Scanned: Debian amavisd-new at disroot.org Content-Transfer-Encoding: quoted-printable DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=disroot.org; s=mail; t=1605210727; bh=gsybXWhhMyOYSFRCqaVzlQwc5CVC07wkIpmhezP9Olw=; h=Subject:From:To:Date:In-Reply-To; b=Bl4urRBRop9EFILOAZyqvH979+vTfZDORWoCkjCkTN/qXWhD270Nxbtxh4rZM4K7a rXJ4Cq0fgQY3rJ0ONU9Mp4At6IJXqLvmcrRTknuofz5hpkRK5KLwLGzneryRq+80hK VJLmsU8MgZ8G7qWnEQRFlwiYolXdIq/VG/weP1OK5y/YXgEfmP+k4DQVUDGYIzd9Ab ojD0GAot3Lt08SxPJVbuRkIMW/6Q3G6ORmpCXqgRgQ8BdRcbYS7cNIwbJWIF00s3yt J37G8qzbRcafPF+6fgQf1Xqrj/DYRfgOgkWOFf4EIiC/3H2iJw5JUkAqlzE0pjhwjR PtsA+NiPXFWlg== Content-Type: text/plain; charset=UTF-8 From: =?utf-8?q?=C3=89rico_Nogueira?= To: , Date: Thu, 12 Nov 2020 16:43:04 -0300 Message-Id: In-Reply-To: <20201112193207.GC2009@voyager> Subject: Re: [musl] [PATCH] fix segfault in lutimes when tv argument is NULL On Thu Nov 12, 2020 at 5:32 PM -03, Markus Wichmann wrote: > On Thu, Nov 12, 2020 at 03:43:27PM -0300, =C3=89rico Nogueira wrote: > > From: =C3=89rico Rolim > > > > calling lutimes with tv=3D0 is valid if the applications wants to set t= he > > timestamps to the current time. short-circuit the function to call > > utimensat with times=3D0 directly if tv =3D=3D 0. > > --- > > > > Bug reported on IRC by nmeum > > > > src/legacy/lutimes.c | 1 + > > 1 file changed, 1 insertion(+) > > > > diff --git a/src/legacy/lutimes.c b/src/legacy/lutimes.c > > index 2e5502d1..22176230 100644 > > --- a/src/legacy/lutimes.c > > +++ b/src/legacy/lutimes.c > > @@ -5,6 +5,7 @@ > > > > int lutimes(const char *filename, const struct timeval tv[2]) > > { > > + if (!tv) return utimensat(AT_FDCWD, filename, 0, AT_SYMLINK_NOFOLLOW)= ; > > struct timespec times[2]; > > times[0].tv_sec =3D tv[0].tv_sec; > > times[0].tv_nsec =3D tv[0].tv_usec * 1000; > > -- > > 2.29.2 > > > > Deja vu. We had a similar discussion in early March. The most recent > e-mail in that thread stated that the patch "might be correct as-is." > Though that patch did attempt to filter out invalid inputs as well. I > had pointed out that the only spec available for lutimes does state that > it should act like utimes(), and utimes() does allow for NULL inputs, > but there was no reply. And no follow-up from the OP, either. > > Ciao, > Markus For reference, that thread starts at https://www.openwall.com/lists/musl/2020/03/01/1 I based myself off of the futime() implementation, so both functions have basically the same look / control flow now (except that futimes() has the `struct timespec times[2]` declaration before the null check, which I can fix in a v2, if necessary). Since it's a legacy function, I didn't think it would be necessary to complicate matters further. Re. checking the input values beyond a NULL check, futime() currently doesn't do it, so for consistency's sake I think it would only make sense to add that verification if it was added to futime() as well. That said, I believe any verification should be left to utimensat(), which seems to be called by most functions in the utimes family. Cheers, =C3=89rico