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.1 required=5.0 tests=DKIM_SIGNED,DKIM_VALID, DKIM_VALID_AU,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 0fc821ce for ; Sun, 1 Mar 2020 20:37:55 +0000 (UTC) Received: (qmail 24258 invoked by uid 550); 1 Mar 2020 20:37:52 -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 24240 invoked from network); 1 Mar 2020 20:37:51 -0000 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=sholland.org; h= subject:to:references:from:message-id:date:mime-version :in-reply-to:content-type:content-transfer-encoding; s=fm2; bh=j gtFPtzPWmma2jMCVP4WjxNeay5I+c7txZjOo9ssey8=; b=Rx0MmEeNQrIepCfOY f2YFqNlYPV6c5h2y1KbZc2TZsJ78abi7crP6rCiQCrODFaDvtxfmqF5AzQ68VRiJ JdsvH0AAnX9k1aYrxnUlbdRRl2LtGdIrL3tI/FgsPT34fuR9AgDrm2IwK9UvD8Cb 5S8d91Z5f41dnogALnrC2rgwyFLnqa4trbsk4WKqEDwMhZrNfE2ZvUY4eCyRJynk zVilHD8eBjz1o9KLLCCbZfqJZ6ODweyvQz8J8ZvWYccDX5FP/iGVKHIEdfQE2KTD Gcd6fK0z78feL2wlofeS3BlgZ5BRJ/kS2A7WPqNlFr70qv+HOQsz3/RuLMyzV841 9TnCg== DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d= messagingengine.com; h=content-transfer-encoding:content-type :date:from:in-reply-to:message-id:mime-version:references :subject:to:x-me-proxy:x-me-proxy:x-me-sender:x-me-sender :x-sasl-enc; s=fm2; bh=jgtFPtzPWmma2jMCVP4WjxNeay5I+c7txZjOo9sse y8=; b=K75XM8AsyF47RRx2MYapV74ncPY6vWDTUGBHXoNcM0uKGQ82WDW1genBo 0aE6Hm6mOajb5DD1fbGrqo5QufxOXjIvPR7gGuvnb29VS9NlCMRPQ5zPSMjZX/Ny sj6rZ2M3l71OOuRmTEVkom7KkKwl9/5AQZuMYPEZvgKXdHsPyD4msWUBdVgiAM6n jcvCqfzH6d7L5nxBgiBykDGrb9XQ6+hHLUfTZsFmJsAGOdfMD3uFgpzQd+YdlUm3 ue4JXrw6vipc0rXWXQ/KWHuPVHqpssvgDo0hOZbQvMoxMpaE3/vMXPsuBPq4otZH NzEByslEBWwOk8LpFZjFtmtW39CFw== X-ME-Sender: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgedugedruddtvddgudefjecutefuodetggdotefrod ftvfcurfhrohhfihhlvgemucfhrghsthforghilhdpqfgfvfdpuffrtefokffrpgfnqfgh necuuegrihhlohhuthemuceftddtnecunecujfgurhepuffvfhfhkffffgggjggtgfesth ejredttdefjeenucfhrhhomhepufgrmhhuvghlucfjohhllhgrnhguuceoshgrmhhuvghl sehshhholhhlrghnugdrohhrgheqnecukfhppeejtddrudefhedrudegkedrudehudenuc evlhhushhtvghrufhiiigvpedtnecurfgrrhgrmhepmhgrihhlfhhrohhmpehsrghmuhgv lhesshhhohhllhgrnhgurdhorhhg X-ME-Proxy: To: musl@lists.openwall.com, Rich Felker References: <20200301065730.146013-1-liujie1@huawei.com> <20200301071737.GI11469@brightrain.aerifal.cx> From: Samuel Holland Message-ID: Date: Sun, 1 Mar 2020 14:37:38 -0600 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.2.2 MIME-Version: 1.0 In-Reply-To: <20200301071737.GI11469@brightrain.aerifal.cx> Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit Subject: Re: [musl] [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 >> --- >> 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. 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