From patchwork Wed Mar 25 07:04:40 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Christophe JAILLET X-Patchwork-Id: 11457063 Return-Path: Received: from mail.kernel.org (pdx-korg-mail-1.web.codeaurora.org [172.30.200.123]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id 15C0A1667 for ; Wed, 25 Mar 2020 07:05:00 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id E957F206F8 for ; Wed, 25 Mar 2020 07:04:59 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1725939AbgCYHE7 (ORCPT ); Wed, 25 Mar 2020 03:04:59 -0400 Received: from smtp02.smtpout.orange.fr ([80.12.242.124]:47645 "EHLO smtp.smtpout.orange.fr" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1725907AbgCYHE7 (ORCPT ); Wed, 25 Mar 2020 03:04:59 -0400 Received: from localhost.localdomain ([93.22.148.147]) by mwinf5d03 with ME id JX4r2200J3B2lW503X4s9d; Wed, 25 Mar 2020 08:04:56 +0100 X-ME-Helo: localhost.localdomain X-ME-Auth: Y2hyaXN0b3BoZS5qYWlsbGV0QHdhbmFkb28uZnI= X-ME-Date: Wed, 25 Mar 2020 08:04:56 +0100 X-ME-IP: 93.22.148.147 From: Christophe JAILLET To: trond.myklebust@hammerspace.com, anna.schumaker@netapp.com, bfields@fieldses.org, chuck.lever@oracle.com, davem@davemloft.net, kuba@kernel.org, gnb@sgi.com, neilb@suse.de, tom@opengridcomputing.com Cc: linux-nfs@vger.kernel.org, linux-kernel@vger.kernel.org, kernel-janitors@vger.kernel.org, Christophe JAILLET Subject: [PATCH 1/2] SUNRPC: Fix a potential buffer overflow in 'svc_print_xprts()' Date: Wed, 25 Mar 2020 08:04:40 +0100 Message-Id: <20200325070440.21988-1-christophe.jaillet@wanadoo.fr> X-Mailer: git-send-email 2.20.1 MIME-Version: 1.0 Sender: linux-nfs-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-nfs@vger.kernel.org 'maxlen' is the total size of the destination buffer. There is only one caller and this value is 256. When we compute the size already used and what we would like to add in the buffer, the trailling NULL character is not taken into account. However, this trailling character will be added by the 'strcat' once we have checked that we have enough place. So, there is a off-by-one issue and 1 byte of the stack could be erroneously overwridden. Take into account the trailling NULL, when checking if there is enough place in the destination buffer. Fixes: dc9a16e49dbba ("svc: Add /proc/sys/sunrpc/transport files") Signed-off-by: Christophe JAILLET --- net/sunrpc/svc_xprt.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/net/sunrpc/svc_xprt.c b/net/sunrpc/svc_xprt.c index d53259346235..df39e7b8b06c 100644 --- a/net/sunrpc/svc_xprt.c +++ b/net/sunrpc/svc_xprt.c @@ -120,7 +120,7 @@ int svc_print_xprts(char *buf, int maxlen) sprintf(tmpstr, "%s %d\n", xcl->xcl_name, xcl->xcl_max_payload); slen = strlen(tmpstr); - if (len + slen > maxlen) + if (len + slen >= maxlen) break; len += slen; strcat(buf, tmpstr); From patchwork Wed Mar 25 07:04:52 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Christophe JAILLET X-Patchwork-Id: 11457065 Return-Path: Received: from mail.kernel.org (pdx-korg-mail-1.web.codeaurora.org [172.30.200.123]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id 176121667 for ; Wed, 25 Mar 2020 07:05:13 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 0234B20B80 for ; Wed, 25 Mar 2020 07:05:13 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1727302AbgCYHFJ (ORCPT ); Wed, 25 Mar 2020 03:05:09 -0400 Received: from smtp02.smtpout.orange.fr ([80.12.242.124]:44712 "EHLO smtp.smtpout.orange.fr" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1727291AbgCYHFI (ORCPT ); Wed, 25 Mar 2020 03:05:08 -0400 Received: from localhost.localdomain ([93.22.148.147]) by mwinf5d03 with ME id JX552200Q3B2lW503X56BU; Wed, 25 Mar 2020 08:05:07 +0100 X-ME-Helo: localhost.localdomain X-ME-Auth: Y2hyaXN0b3BoZS5qYWlsbGV0QHdhbmFkb28uZnI= X-ME-Date: Wed, 25 Mar 2020 08:05:07 +0100 X-ME-IP: 93.22.148.147 From: Christophe JAILLET To: trond.myklebust@hammerspace.com, anna.schumaker@netapp.com, bfields@fieldses.org, chuck.lever@oracle.com, davem@davemloft.net, kuba@kernel.org, gnb@sgi.com, neilb@suse.de, tom@opengridcomputing.com Cc: linux-nfs@vger.kernel.org, linux-kernel@vger.kernel.org, kernel-janitors@vger.kernel.org, Christophe JAILLET Subject: [PATCH 2/2] SUNRPC: Optimize 'svc_print_xprts()' Date: Wed, 25 Mar 2020 08:04:52 +0100 Message-Id: <20200325070452.22043-1-christophe.jaillet@wanadoo.fr> X-Mailer: git-send-email 2.20.1 MIME-Version: 1.0 Sender: linux-nfs-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-nfs@vger.kernel.org Using 'snprintf' is safer than 'sprintf' because it can avoid a buffer overflow. The return value can also be used to avoid a strlen a call. Finally, we know where we need to copy and the length to copy, so, we can save a few cycles by rearraging the code and using a memcpy instead of a strcat. Signed-off-by: Christophe JAILLET --- This patch should have no functionnal change. We could go further, use scnprintf and write directly in the destination buffer. However, this could lead to a truncated last line. --- net/sunrpc/svc_xprt.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/net/sunrpc/svc_xprt.c b/net/sunrpc/svc_xprt.c index df39e7b8b06c..6df861650040 100644 --- a/net/sunrpc/svc_xprt.c +++ b/net/sunrpc/svc_xprt.c @@ -118,12 +118,12 @@ int svc_print_xprts(char *buf, int maxlen) list_for_each_entry(xcl, &svc_xprt_class_list, xcl_list) { int slen; - sprintf(tmpstr, "%s %d\n", xcl->xcl_name, xcl->xcl_max_payload); - slen = strlen(tmpstr); - if (len + slen >= maxlen) + slen = snprintf(tmpstr, sizeof(tmpstr), "%s %d\n", + xcl->xcl_name, xcl->xcl_max_payload); + if (slen >= sizeof(tmpstr) || len + slen >= maxlen) break; + memcpy(buf + len, tmpstr, slen + 1); len += slen; - strcat(buf, tmpstr); } spin_unlock(&svc_xprt_class_lock);