From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from loongson.cn (mail.loongson.cn [114.242.206.163]) by sourceware.org (Postfix) with ESMTP id 1AA0E3858C41 for ; Sat, 20 May 2023 00:29:48 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 1AA0E3858C41 Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=loongson.cn Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=loongson.cn Received: from loongson.cn (unknown [111.9.175.10]) by gateway (Coremail) with SMTP id _____8DxZPB5FGhkD08KAA--.18017S3; Sat, 20 May 2023 08:29:46 +0800 (CST) Received: from [10.136.13.28] (unknown [111.9.175.10]) by localhost.localdomain (Coremail) with SMTP id AQAAf8CxJLF3FGhk8elqAA--.50556S3; Sat, 20 May 2023 08:29:45 +0800 (CST) Subject: Re: [PATCH] Increase judgment on buf. To: Carlos O'Donell , Adhemerval Zanella Netto , Peng Fan , libc-alpha@sourceware.org Cc: Xi Ruoyao References: <20230519035713.3453563-1-fanpeng@loongson.cn> <374d9752-5a9b-15b2-058f-62943e6c7a33@linaro.org> <293741fd-f97a-0b23-6487-20253c4608fb@redhat.com> From: lixing Message-ID: <638972db-f366-4caf-f60d-92db34dd1b80@loongson.cn> Date: Sat, 20 May 2023 08:29:43 +0800 User-Agent: Mozilla/5.0 (X11; Linux loongarch64; rv:68.0) Gecko/20100101 Thunderbird/68.7.0 MIME-Version: 1.0 In-Reply-To: <293741fd-f97a-0b23-6487-20253c4608fb@redhat.com> Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 8bit Content-Language: en-US X-CM-TRANSID:AQAAf8CxJLF3FGhk8elqAA--.50556S3 X-CM-SenderInfo: pol0x03j6o00pqjv00gofq/ X-Coremail-Antispam: 1Uk129KBjvJXoW7WF1DKF13tFyUCFyDXFyrZwb_yoW8ZFWUpF 47C3WjkF4UJw4xGw1a9w1DZr1xWa9xtrWDurs0g3WYvasrZwnaka1qkrWSgrnFvryfWa18 ZrWDK3Z7Wa98Ja7anT9S1TB71UUUUUJqnTZGkaVYY2UrUUUUj1kv1TuYvTs0mT0YCTnIWj qI5I8CrVACY4xI64kE6c02F40Ex7xfYxn0WfASr-VFAUDa7-sFnT9fnUUIcSsGvfJTRUUU bf8YFVCjjxCrM7AC8VAFwI0_Jr0_Gr1l1xkIjI8I6I8E6xAIw20EY4v20xvaj40_Wr0E3s 1l1IIY67AEw4v_Jr0_Jr4l8cAvFVAK0II2c7xJM28CjxkF64kEwVA0rcxSw2x7M28EF7xv wVC0I7IYx2IY67AKxVWUJVWUCwA2z4x0Y4vE2Ix0cI8IcVCY1x0267AKxVWUJVW8JwA2z4 x0Y4vEx4A2jsIE14v26r4j6F4UM28EF7xvwVC2z280aVCY1x0267AKxVW8JVW8Jr1ln4kS 14v26r1Y6r17M2AIxVAIcxkEcVAq07x20xvEncxIr21l57IF6xkI12xvs2x26I8E6xACxx 1l5I8CrVACY4xI64kE6c02F40Ex7xfMcIj6xIIjxv20xvE14v26r1Y6r17McIj6I8E87Iv 67AKxVWUJVW8JwAm72CE4IkC6x0Yz7v_Jr0_Gr1lF7xvr2IY64vIr41lc7I2V7IY0VAS07 AlzVAYIcxG8wCF04k20xvY0x0EwIxGrwCFx2IqxVCFs4IE7xkEbVWUJVW8JwCFI7km07C2 67AKxVWUXVWUAwC20s026c02F40E14v26r1j6r18MI8I3I0E7480Y4vE14v26r106r1rMI 8E67AF67kF1VAFwI0_JF0_Jw1lIxkGc2Ij64vIr41lIxAIcVC0I7IYx2IY67AKxVWUJVWU CwCI42IY6xIIjxv20xvEc7CjxVAFwI0_Jr0_Gr1lIxAIcVCF04k26cxKx2IYs7xG6r1j6r 1xMIIF0xvEx4A2jsIE14v26r1j6r4UMIIF0xvEx4A2jsIEc7CjxVAFwI0_Jr0_GrUvcSsG vfC2KfnxnUUI43ZEXa7IU88Ma5UUUUU== X-Spam-Status: No, score=-11.2 required=5.0 tests=BAYES_00,GIT_PATCH_0,KAM_DMARC_STATUS,NICE_REPLY_A,RCVD_IN_SBL_CSS,SPF_HELO_PASS,SPF_PASS,TXREP,T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org List-Id: 在 2023/5/19 下午7:55, Carlos O'Donell via Libc-alpha 写道: > On 5/19/23 07:48, Adhemerval Zanella Netto via Libc-alpha wrote: >> >> On 19/05/23 00:57, Peng Fan wrote: >>> When buf is empty, if it is not checked, the subsequent assignment >>> operation will trigger a page fault. This is unnecessary. >>> >>> Signed-off-by: lixing >>> Signed-off-by: Peng Fan >> The stat family is explicitly marked with nonnull for the input struct >> stat buffer, and calling with a NULL argument is an UB. > Agreed, and "Style and Conventions" > https://sourceware.org/glibc/wiki/Style_and_Conventions > says: > https://sourceware.org/glibc/wiki/Style_and_Conventions#Bugs_in_the_user_program > > We should fail catastrophically and early in the case of user bugs. > The segfault generates a core dump at exactly the right point to debug the UB. Yes, LTP fstat03 test pass the buf with NULL. We just want to fail earlier during the syscall statx with return value EFAULT if the buf is NULL, but not pass the fault to the struct assignment which trigger SIGSEGV. >>> --- >>> sysdeps/unix/sysv/linux/fstatat64.c | 8 ++++++-- >>> 1 file changed, 6 insertions(+), 2 deletions(-) >>> >>> diff --git a/sysdeps/unix/sysv/linux/fstatat64.c b/sysdeps/unix/sysv/linux/fstatat64.c >>> index 3509d3ca6d..b635a8299a 100644 >>> --- a/sysdeps/unix/sysv/linux/fstatat64.c >>> +++ b/sysdeps/unix/sysv/linux/fstatat64.c >>> @@ -52,9 +52,13 @@ fstatat64_time64_statx (int fd, const char *file, struct __stat64_t64 *buf, >>> { >>> /* 32-bit kABI with default 64-bit time_t, e.g. arc, riscv32. Also >>> 64-bit time_t support is done through statx syscall. */ >>> - struct statx tmp; >>> + struct statx tmp, *ptr; >>> + if (buf) >>> + ptr = &tmp; >>> + else >>> + ptr = NULL; >>> int r = INTERNAL_SYSCALL_CALL (statx, fd, file, AT_NO_AUTOMOUNT | flag, >>> - STATX_BASIC_STATS, &tmp); >>> + STATX_BASIC_STATS, ptr); >>> if (r != 0) >>> return r; >>>