From patchwork Mon Nov 23 00:35:54 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Samuel Thibault X-Patchwork-Id: 41145 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 F0FFC385780C; Mon, 23 Nov 2020 00:36:02 +0000 (GMT) X-Original-To: libc-alpha@sourceware.org Delivered-To: libc-alpha@sourceware.org Received: from hera.aquilenet.fr (hera.aquilenet.fr [185.233.100.1]) by sourceware.org (Postfix) with ESMTPS id C1A9C3858008 for ; Mon, 23 Nov 2020 00:35:59 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org C1A9C3858008 Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=ens-lyon.org Authentication-Results: sourceware.org; spf=fail smtp.mailfrom=samuel.thibault@ens-lyon.org Received: from localhost (localhost [127.0.0.1]) by hera.aquilenet.fr (Postfix) with ESMTP id 7A58D119A; Mon, 23 Nov 2020 01:35:58 +0100 (CET) X-Virus-Scanned: Debian amavisd-new at aquilenet.fr Received: from hera.aquilenet.fr ([127.0.0.1]) by localhost (hera.aquilenet.fr [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id P9YvfrwpIF-Q; Mon, 23 Nov 2020 01:35:56 +0100 (CET) Received: from function.youpi.perso.aquilenet.fr (lfbn-bor-1-56-204.w90-50.abo.wanadoo.fr [90.50.148.204]) by hera.aquilenet.fr (Postfix) with ESMTPSA id A598AF00; Mon, 23 Nov 2020 01:35:56 +0100 (CET) Received: from samy by function.youpi.perso.aquilenet.fr with local (Exim 4.94) (envelope-from ) id 1kgzpj-006LfC-EY; Mon, 23 Nov 2020 01:35:55 +0100 From: Samuel Thibault To: libc-alpha@sourceware.org Subject: [hurd,commited 5/5] hurd report-wait: Fix stpcpy usage Date: Mon, 23 Nov 2020 01:35:54 +0100 Message-Id: <20201123003554.1513184-6-samuel.thibault@ens-lyon.org> X-Mailer: git-send-email 2.29.2 In-Reply-To: <20201123003554.1513184-1-samuel.thibault@ens-lyon.org> References: <20201123003554.1513184-1-samuel.thibault@ens-lyon.org> MIME-Version: 1.0 X-Spam-Status: No, score=-11.4 required=5.0 tests=BAYES_00, GIT_PATCH_0, JMQ_SPF_NEUTRAL, KAM_DMARC_STATUS, SPF_HELO_PASS, SPF_NEUTRAL, TXREP autolearn=ham autolearn_force=no version=3.4.2 X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on server2.sourceware.org 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: , Cc: commit-hurd@gnu.org Errors-To: libc-alpha-bounces@sourceware.org Sender: "Libc-alpha" We shall not overflow the size of the description parameter. This makes describe_number and describe_port behave like strpcpy (except for not filling all the end of buffer with zeroes) and _S_msg_report_wait use series of stpncpy-like call. If we were to overflow, we can now detect it and return ENOMEM. --- hurd/report-wait.c | 78 +++++++++++++++++++++++++++++----------------- 1 file changed, 49 insertions(+), 29 deletions(-) diff --git a/hurd/report-wait.c b/hurd/report-wait.c index eba43c97a6..291032142a 100644 --- a/hurd/report-wait.c +++ b/hurd/report-wait.c @@ -26,12 +26,16 @@ #include static char * -describe_number (string_t description, const char *flavor, long int i) +describe_number (char *description, const char *flavor, long int i, size_t size) { unsigned long int j; - char *p = flavor == NULL ? description : __stpcpy (description, flavor); + char *limit = description + size; + char *p = flavor == NULL ? description : __stpncpy (description, flavor, size); char *end; + if (p == limit) + return p; + /* Handle sign. */ if (i < 0) { @@ -39,15 +43,24 @@ describe_number (string_t description, const char *flavor, long int i) *p++ = '-'; } + if (p == limit) + return p; + /* Allocate space for the number at the end of DESCRIPTION. */ for (j = i; j >= 10; j /= 10) p++; end = p + 1; - *end = '\0'; + + if (end < limit) + *end = '\0'; + else + end = limit; do { - *p-- = '0' + i % 10; + if (p < limit) + *p = '0' + i % 10; + p--; i /= 10; } while (i != 0); @@ -55,27 +68,27 @@ describe_number (string_t description, const char *flavor, long int i) } static char * -describe_port (string_t description, mach_port_t port) +describe_port (char *description, mach_port_t port, size_t size) { int i; if (port == MACH_PORT_NULL) - return __stpcpy (description, "(null)"); + return __stpncpy (description, "(null)", size); if (port == MACH_PORT_DEAD) - return __stpcpy (description, "(dead)"); + return __stpncpy (description, "(dead)", size); if (port == __mach_task_self ()) - return __stpcpy (description, "task-self"); + return __stpncpy (description, "task-self", size); for (i = 0; i < _hurd_nports; ++i) if (port == _hurd_ports[i].port) - return describe_number (description, "init#", i); + return describe_number (description, "init#", i, size); if (_hurd_init_dtable) { for (i = 0; i < _hurd_init_dtablesize; ++i) if (port == _hurd_init_dtable[i]) - return describe_number (description, "fd#", i); + return describe_number (description, "fd#", i, size); } if (_hurd_dtable) { @@ -83,12 +96,12 @@ describe_port (string_t description, mach_port_t port) if (_hurd_dtable[i] == NULL) continue; else if (port == _hurd_dtable[i]->port.port) - return describe_number (description, "fd#", i); + return describe_number (description, "fd#", i, size); else if (port == _hurd_dtable[i]->ctty.port) - return describe_number (description, "bgfd#", i); + return describe_number (description, "bgfd#", i, size); } - return describe_number (description, "port#", port); + return describe_number (description, "port#", port, size); } @@ -106,13 +119,15 @@ kern_return_t _S_msg_report_wait (mach_port_t msgport, thread_t thread, string_t description, mach_msg_id_t *msgid) { + char *limit = description + 1024; /* XXX */ + char *cur = description; *msgid = 0; if (thread == _hurd_msgport_thread) /* Cute. */ - strcpy (description, "msgport"); + cur = __stpncpy (cur, "msgport", limit - cur); else if (&_hurd_itimer_thread && thread == _hurd_itimer_thread) - strcpy (description, "itimer"); + cur = __stpncpy (cur, "itimer", limit - cur); else { /* Make sure this is really one of our threads. */ @@ -129,7 +144,7 @@ _S_msg_report_wait (mach_port_t msgport, thread_t thread, return EINVAL; if (ss->suspended != MACH_PORT_NULL) - strcpy (description, "sigsuspend"); + cur = __stpncpy (cur, "sigsuspend", limit - cur); else { /* Examine the thread's state to see if it is blocked in an RPC. */ @@ -155,7 +170,6 @@ _S_msg_report_wait (mach_port_t msgport, thread_t thread, && MSG_EXAMINE (&state, msgid, &rcv_port, &send_port, &option, &timeout) == 0) { - char *p; if (send_port != MACH_PORT_NULL && *msgid != 0) { /* For the normal case of RPCs, we consider the @@ -168,13 +182,14 @@ _S_msg_report_wait (mach_port_t msgport, thread_t thread, /* This is a Hurd interruptible RPC. Mark it by surrounding the port description string with [...] brackets. */ - description[0] = '['; - p = describe_port (description + 1, send_port); - *p++ = ']'; - *p = '\0'; + if (cur < limit) + *cur++ = '['; + cur = describe_port (cur, send_port, limit - cur); + if (cur < limit) + *cur++ = ']'; } else - (void) describe_port (description, send_port); + cur = describe_port (cur, send_port, limit - cur); } else if (rcv_port != MACH_PORT_NULL) { @@ -183,7 +198,8 @@ _S_msg_report_wait (mach_port_t msgport, thread_t thread, some garbage or perhaps the msgid of the last message this thread received, but it's not a helpful thing to return. */ - strcpy (describe_port (description, rcv_port), ":rcv"); + cur = describe_port (cur, rcv_port, limit - cur); + cur = __stpncpy (cur, ":rcv", limit - cur); *msgid = 0; } else if ((option & (MACH_RCV_MSG|MACH_RCV_TIMEOUT)) @@ -193,27 +209,31 @@ _S_msg_report_wait (mach_port_t msgport, thread_t thread, pure timeout. Report the timeout value (counted in milliseconds); note this is the original total time, not the time remaining. */ - strcpy (describe_number (description, 0, timeout), "ms"); + cur = describe_number (cur, 0, timeout, limit - cur); + cur = __stpncpy (cur, "ms", limit - cur); *msgid = 0; } else { - strcpy (description, "mach_msg"); + cur = __stpncpy (cur, "mach_msg", limit - cur); *msgid = 0; } } else /* Some other system call. */ { - (void) describe_number (description, "syscall#", *msgid); + cur = describe_number (cur, "syscall#", *msgid, limit - cur); *msgid = 0; } } - else - description[0] = '\0'; } } __mach_port_deallocate (__mach_task_self (), thread); + + if (cur == limit) + return ENOMEM; + + *cur = '\0'; return 0; } @@ -232,7 +252,7 @@ _S_msg_describe_ports (mach_port_t msgport, mach_port_t refport, while (nports-- > 0) { char this[200]; - describe_port (this, *ports++); + describe_port (this, *ports++, sizeof this); p = __stpncpy (p, this, end - p); if (p == end && p[-1] != '\0') return ENOMEM;