From patchwork Fri Aug 25 02:49:59 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: "zhanghao \\(ES\\)" X-Patchwork-Id: 74693 Return-Path: X-Original-To: patchwork@sourceware.org Delivered-To: patchwork@sourceware.org Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 217E23858C2B for ; Fri, 25 Aug 2023 02:50:34 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 217E23858C2B DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=sourceware.org; s=default; t=1692931834; bh=qKV2OyMSqtJ8RI2jrZALACu/ih/KhpE+KQ4S0laTzS0=; h=To:CC:Subject:Date:References:In-Reply-To:List-Id: List-Unsubscribe:List-Archive:List-Post:List-Help:List-Subscribe: From:Reply-To:From; b=wvL1G2n8LeHkRhoQcxUKFs2mifRA5H0gfs/SyJhYunjeNitUYtrdB+vJdzKik3ded mvvLaeHFc4+8Dpf73y0xW9MpBGGPhTezHBWffFw7QbuMA+Ykv+Sg+DDwxOZ8Za1F0L G+KxQEjRlQC6KCKlkFAnyWZSQDQzY6BIPRZ3+D20= X-Original-To: libc-alpha@sourceware.org Delivered-To: libc-alpha@sourceware.org Received: from szxga02-in.huawei.com (szxga02-in.huawei.com [45.249.212.188]) by sourceware.org (Postfix) with ESMTPS id 97D7C3858C53 for ; Fri, 25 Aug 2023 02:50:03 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 97D7C3858C53 Received: from kwepemm600003.china.huawei.com (unknown [172.30.72.56]) by szxga02-in.huawei.com (SkyGuard) with ESMTP id 4RX48c4PFHzNn27 for ; Fri, 25 Aug 2023 10:46:24 +0800 (CST) Received: from kwepemm600004.china.huawei.com (7.193.23.242) by kwepemm600003.china.huawei.com (7.193.23.202) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2507.31; Fri, 25 Aug 2023 10:49:59 +0800 Received: from kwepemm600004.china.huawei.com ([7.193.23.242]) by kwepemm600004.china.huawei.com ([7.193.23.242]) with mapi id 15.01.2507.031; Fri, 25 Aug 2023 10:49:59 +0800 To: "libc-alpha@sourceware.org" CC: "Yanan (Euler)" , Caowangbao , liaichun , "chenhaixiang (A)" Subject: =?eucgb2312_cn?b?16q3ojogYXZvaWQgc25wcmludGYgdXNpbmcgJW4gdG8gZ2VuZXJhdGUg?= =?eucgb2312_cn?b?Y29yZWR1bXAgd2hlbiBGX1M9MiBpcyBlbmFibGVk?= Thread-Topic: avoid snprintf using %n to generate coredump when F_S=2 is enabled Thread-Index: AdnUC4TR1OXiZJR0SFSeMtBAsJoI8QC8ss0A Date: Fri, 25 Aug 2023 02:49:59 +0000 Message-ID: <8f63a5a69ee84f38b4e73ae5b5ff38ef@huawei.com> References: <4bce41a8bc934ae4993c9e50d43d4bc0@huawei.com> In-Reply-To: <4bce41a8bc934ae4993c9e50d43d4bc0@huawei.com> Accept-Language: zh-CN, en-US Content-Language: zh-CN X-MS-Has-Attach: yes X-MS-TNEF-Correlator: x-originating-ip: [10.136.118.161] MIME-Version: 1.0 X-CFilter-Loop: Reflected X-Spam-Status: No, score=3.3 required=5.0 tests=BAYES_00, BODY_8BITS, CHARSET_FARAWAY_HEADER, GIT_PATCH_0, HTML_MESSAGE, IMAGE_ATTACHED, KAM_DMARC_STATUS, KAM_LOTSOFHASH, MIME_CHARSET_FARAWAY, RCVD_IN_MSPIKE_H5, RCVD_IN_MSPIKE_WL, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Level: *** X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org X-Content-Filtered-By: Mailman/MimeDel 2.1.29 X-BeenThere: libc-alpha@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Libc-alpha mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-Patchwork-Original-From: "zhanghao \(ES\) via Libc-alpha" From: "zhanghao \\(ES\\)" Reply-To: "zhanghao \(ES\)" Errors-To: libc-alpha-bounces+patchwork=sourceware.org@sourceware.org Sender: "Libc-alpha" Bug 30795: https://sourceware.org/bugzilla/show_bug.cgi?id=30795 The patch to solve the problem is as follows: From 4816192ca348e55b7b1d33feac9298d5b0ffb04c Mon Sep 17 00:00:00 2001 From: zhanghao Date: Mon, 21 Aug 2023 15:39:56 +0800 Subject: [PATCH] Avoid snprintf using %n to generate coredump when F_S=2 is enabled In nscd, F_S=2 added in 233399bce2e79e5af3b344782e9943d5f1a9cdcb just for warn_if_unused warnings rather than anything substantial. When F_S=2 is set, and snprintf() using %n will generate coredump and give the following prompt: *** %n in writable segment detected *** It is not recommended to use %n to calculate the length of the string in the snprintf function. We strip the calculation logic outside the snprintf function for replacement. --- nscd/grpcache.c | 5 +++-- nscd/pwdcache.c | 5 +++-- 2 files changed, 6 insertions(+), 4 deletions(-) -- 2.33.0 diff --git a/nscd/grpcache.c b/nscd/grpcache.c index 457ca4d8..d7200f4e 100644 --- a/nscd/grpcache.c +++ b/nscd/grpcache.c @@ -176,8 +176,9 @@ cache_addgr (struct database_dyn *db, int fd, request_header *req, /* We need this to insert the `bygid' entry. */ int key_offset; - n = snprintf (buf, buf_len, "%d%c%n%s", grp->gr_gid, '\0', - &key_offset, (char *) key) + 1; + n = snprintf (buf, buf_len, "%d%c%s", grp->gr_gid, '\0', + (char *) key) + 1; + key_offset = n - strlen((char *) key)- 1; /* Determine the length of all members. */ while (grp->gr_mem[gr_mem_cnt]) diff --git a/nscd/pwdcache.c b/nscd/pwdcache.c index dfafb526..37dd402f 100644 --- a/nscd/pwdcache.c +++ b/nscd/pwdcache.c @@ -180,8 +180,9 @@ cache_addpw (struct database_dyn *db, int fd, request_header *req, /* We need this to insert the `byuid' entry. */ int key_offset; - n = snprintf (buf, buf_len, "%d%c%n%s", pwd->pw_uid, '\0', - &key_offset, (char *) key) + 1; + n = snprintf (buf, buf_len, "%d%c%s", pwd->pw_uid, '\0', + (char *) key) + 1; + key_offset = n - strlen((char *) key) - 1; total = (offsetof (struct dataset, strdata) + pw_name_len + pw_passwd_len -- 2.33.0 发件人: zhanghao (ES) 发送时间: 2023年8月21日 16:47 收件人: 'admin-requests@sourceware.org' 抄送: Yanan (Euler) ; Caowangbao ; liaichun ; chenhaixiang (A) 主题: avoid snprintf using %n to generate coredump when F_S=2 is enabled Dear all, Recently, we found that two coredump occurred when nscd involved calling the snprintf function and using %n and F_S=2 is set, the following two call stacks: Coredump1: [cid:image003.jpg@01D9D44E.FBA59250] Coredump2: [cid:image005.jpg@01D9D44E.FBA59250] and give the following prompt: *** %n in writable segment detected *** And the input parameters of the two call stacks look normal. Involved version: glibc 2.34 We use a simple test case to verify it: #include #include int main () { char fmtstring[10]; char buf[100]; int count = -1; strcpy (fmtstring, "%d%n"); snprintf (buf, 100, fmtstring, 123, &count); return 0; } when compiling with gcc snprintf_test.c -fstack-protector -Wall -Wformat -Wformat-security -D_FORTIFY_SOURCE=2 -O2 -o snprintf_test -g ./ snprintf_test *** %n in writable segment detected *** Aborted (core dumped) when compiling with gcc snprintf_test.c -fstack-protector -Wall -Wformat -Wformat-security -O2 -o snprintf_test -g ./ snprintf_test no core dumped We strip the calculation logic outside the snprintf function for replacement: From 4816192ca348e55b7b1d33feac9298d5b0ffb04c Mon Sep 17 00:00:00 2001 From: zhanghao> Date: Mon, 21 Aug 2023 15:39:56 +0800 Subject: [PATCH] Avoid snprintf using %n to generate coredump when F_S=2 is enabled In nscd, F_S=2 added in 233399bce2e79e5af3b344782e9943d5f1a9cdcb just for warn_if_unused warnings rather than anything substantial. When F_S=2 is set, and snprintf() using %n will generate coredump and give the following prompt: *** %n in writable segment detected *** It is not recommended to use %n to calculate the length of the string in the snprintf function. We strip the calculation logic outside the snprintf function for replacement. --- nscd/grpcache.c | 5 +++-- nscd/pwdcache.c | 5 +++-- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/nscd/grpcache.c b/nscd/grpcache.c index 457ca4d8..d7200f4e 100644 --- a/nscd/grpcache.c +++ b/nscd/grpcache.c @@ -176,8 +176,9 @@ cache_addgr (struct database_dyn *db, int fd, request_header *req, /* We need this to insert the `bygid' entry. */ int key_offset; - n = snprintf (buf, buf_len, "%d%c%n%s", grp->gr_gid, '\0', - &key_offset, (char *) key) + 1; + n = snprintf (buf, buf_len, "%d%c%s", grp->gr_gid, '\0', + (char *) key) + 1; + key_offset = n - strlen((char *) key)- 1; /* Determine the length of all members. */ while (grp->gr_mem[gr_mem_cnt]) diff --git a/nscd/pwdcache.c b/nscd/pwdcache.c index dfafb526..37dd402f 100644 --- a/nscd/pwdcache.c +++ b/nscd/pwdcache.c @@ -180,8 +180,9 @@ cache_addpw (struct database_dyn *db, int fd, request_header *req, /* We need this to insert the `byuid' entry. */ int key_offset; - n = snprintf (buf, buf_len, "%d%c%n%s", pwd->pw_uid, '\0', - &key_offset, (char *) key) + 1; + n = snprintf (buf, buf_len, "%d%c%s", pwd->pw_uid, '\0', + (char *) key) + 1; + key_offset = n - strlen((char *) key) - 1; total = (offsetof (struct dataset, strdata) + pw_name_len + pw_passwd_len