From patchwork Tue Aug 1 12:04:03 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Arnd Bergmann X-Patchwork-Id: 109141 Delivered-To: patch@linaro.org Received: by 10.140.101.6 with SMTP id t6csp1408032qge; Tue, 1 Aug 2017 05:05:09 -0700 (PDT) X-Received: by 10.84.132.104 with SMTP id 95mr20737254ple.228.1501589109105; Tue, 01 Aug 2017 05:05:09 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1501589109; cv=none; d=google.com; s=arc-20160816; b=B5ooxaKGqlefS8pnXuZBXAHmtjUyRvuvcTtCNfxV2lrGQ5frG5ycZbZ1ISlFxwYsXF eKHsdCeqHmd/DRzCUglDCFBaop8lxIWdJ9f67L805JwIMovJJ1Y2RNIcmLAwOp/nXepm qBzhyMAVlPvUvHaknEjaIYerL5V/UtnpBDME47jJQtLRZ0Y9dW9UGj0CVSYzxvcmFxuF T0J85L/F9zD3L/pw5BuVbYGS0MznlldublUaeFtwjEa0OldgsGZXcWCptePsritW6Bzt zyGDI0LTF2FtkJaAYvlPN4zLlt1Ucnn1Ydi5i6AzMJ2JUm2XZlkc4If20+o0NXH0ENvn 5hHQ== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:sender:references:in-reply-to:message-id:date :subject:cc:to:from:arc-authentication-results; bh=KGt0yBPoMNLnm3Ubkn8TJxppqoLRdOKSt2YmUX1zeEs=; b=JUHldu7HS+8jzpNvjO14Dp76Q5xF1mDDNdIccYfNsIa4aqV8hYMAntZso/IID80w9N RQsDIdPJ4RzMunqpaB6u9fIcsW3xpHMkM6N2TBvEVpe2+S7VeHC8Od95oxeSr9lmOwJC /FYZPUNFE9j6Xlk1A4RI5t/veB7wS0yFIAozbNSbqSk2wnDeZ8KYNLsDbErxpPfuJYmG 91A+t/i0ShPhIWvFyo/tNxfmEGcYsKomYtp34mnOW6d31UfY8szO7PiyfrFLFenGz7Vn d2jCjS1Rvc2v4Fv+fueJ1dOXmnJ0FbIhRJ0YpyGA7+E4a8aMealfiLC5BuW6NSjugY/V Kt0w== ARC-Authentication-Results: i=1; mx.google.com; spf=pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org Return-Path: Received: from vger.kernel.org (vger.kernel.org. [209.132.180.67]) by mx.google.com with ESMTP id o79si13758909pfi.625.2017.08.01.05.05.08; Tue, 01 Aug 2017 05:05:09 -0700 (PDT) Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67; Authentication-Results: mx.google.com; spf=pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751755AbdHAMFE (ORCPT + 26 others); Tue, 1 Aug 2017 08:05:04 -0400 Received: from mout.kundenserver.de ([217.72.192.75]:50342 "EHLO mout.kundenserver.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751115AbdHAMFD (ORCPT ); Tue, 1 Aug 2017 08:05:03 -0400 Received: from wuerfel.lan ([78.43.238.10]) by mrelayeu.kundenserver.de (mreue103 [212.227.15.145]) with ESMTPA (Nemesis) id 0M4kN3-1dSxoB0SlB-00yvlu; Tue, 01 Aug 2017 14:04:46 +0200 From: Arnd Bergmann To: "Theodore Ts'o" , Andreas Dilger Cc: Kees Cook , Andrew Morton , Arnd Bergmann , Jan Kara , Chandan Rajendra , linux-ext4@vger.kernel.org, linux-kernel@vger.kernel.org Subject: [PATCH 1/2] ext4: fix warning about stack corruption Date: Tue, 1 Aug 2017 14:04:03 +0200 Message-Id: <20170801120438.1582336-1-arnd@arndb.de> X-Mailer: git-send-email 2.9.0 In-Reply-To: <20170726185219.GA57833@beast> References: <20170726185219.GA57833@beast> X-Provags-ID: V03:K0:AT02oLEiWlYjS2SkQHrFciGNwP0lOoIaeubS7Rq93rcYlDa0shs zMO//xgxbGFom3XwYZJF44FsYdBIyBqFUSXMrvg9Sjfmi9qr02zdNX65I7/DxrdeTJ4bt3z Ll5syxiYcQoh9Euzsvkvw+OsvjHdFIGlVskG8Ekc3zSBp2fFdaZrNOn/F7MH0tK4n+ohYgu 2nMFP/GarqTckBiXi+3Mw== X-UI-Out-Filterresults: notjunk:1; V01:K0:wMPo3xTIwe4=:L6rJPiGZk29Osk0ruO/jAw Qeek8cL4/WcMWfowkEnW5W5yjFl2PGaez1/Rs8khbEEbNkN+ZWBXFD8kYEGfWjtVDnzAK5SVy HhmrQG8dvocVrnfJfEI6bDI5ht9kgR0iJvLEjrSfRu9fnbVikuVS5abwsv3+8GrfHEsw3Hq5Z lg+WGS+JAHg2Py+GEx/rHsXWPuQBYyh3yRSRBP3TNk7AN7bBg8P46/+f4KuQ5Khme0XGdwF4H VVWaXBOT7Xam4VY2rcHjKOgiwiv9lsdYOcSM83Dt819iyXLO/8RlRxJ7XAGW+6cRA8FvR67fS udNbCv3TNoiFbil+xFublc11MAGOa7YmJJcVftfyox2d6vFIcDxrANvTB78kArgqQ7ML8wi+z 0ymV/j/wdJM2Lh5XhMx+SPnVaflvdAvBAPJE80gWMdqidvGOk9EjTiE+l/4ZGdfHZwxQgHaB4 3KkSNicVpptdIhALaOeUyUhuXJPQ+Uq4Xg+w8a3AMTgDdfX6RjGE44dIN9Wgz9mPCthvHkVfh 4qsdwMcLfq1asks7LmKGWSrmJsAVLBYjIoMsnr9wcBVck3Vx4FWX7+S+dqT2/phYYeFZJzZ5a T91Wi+RxXK4Lj1gk8xaXSo/aqhKBdws8U/mviAhShpQXW1FB7huK29UhsS2aXkojGK2f5Pxp8 XKwhY8TAAIAFX3+5I5vy08kUtHf3mHNQvV0P8dMo1TUjxzsBxDapHAi07Pe9ECLx5Jg9itkJT k3FgY9HzQcRNkUxvh6113YiQ5OU5blDrV/6hHw== Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org After commit 62d1034f53e3 ("fortify: use WARN instead of BUG for now"), we get a warning about possible stack overflow from a memcpy that was not strictly bounded to the size of the local variable: inlined from 'ext4_mb_seq_groups_show' at fs/ext4/mballoc.c:2322:2: include/linux/string.h:309:9: error: '__builtin_memcpy': writing between 161 and 1116 bytes into a region of size 160 overflows the destination [-Werror=stringop-overflow=] We actually had a bug here that would have been found by the warning, but it was already fixed last year in commit 30a9d7afe70e ("ext4: fix stack memory corruption with 64k block size"). This replaces the fixed-length structure on the stack with a variable-length structure, using the correct upper bound that tells the compiler that everything is really fine here. I also change the loop count to check for the same upper bound for consistency, but the existing code is already correct here. Note that while clang won't allow certain kinds of variable-length arrays in structures, this particular instance is fine, as the array is at the end of the structure, and the size is strictly bounded. There is one remaining issue with the function that I'm not addressing here: With s_blocksize_bits==16, we don't actually print the last two members of the array, as we loop though just the first 14 members. This could be easily addressed by adding two extra columns in the output, but that could in theory break parsers in user space, and should be a separate patch if we decide to modify it. Signed-off-by: Arnd Bergmann --- fs/ext4/mballoc.c | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) -- 2.9.0 Acked-by: Kees Cook Tested-by: Chandan Rajendra diff --git a/fs/ext4/mballoc.c b/fs/ext4/mballoc.c index 581e357e8406..803cab1939fe 100644 --- a/fs/ext4/mballoc.c +++ b/fs/ext4/mballoc.c @@ -2295,9 +2295,12 @@ static int ext4_mb_seq_groups_show(struct seq_file *seq, void *v) int err, buddy_loaded = 0; struct ext4_buddy e4b; struct ext4_group_info *grinfo; + unsigned char blocksize_bits = min_t(unsigned char, + sb->s_blocksize_bits, + EXT4_MAX_BLOCK_LOG_SIZE); struct sg { struct ext4_group_info info; - ext4_grpblk_t counters[EXT4_MAX_BLOCK_LOG_SIZE + 2]; + ext4_grpblk_t counters[blocksize_bits + 2]; } sg; group--; @@ -2306,8 +2309,6 @@ static int ext4_mb_seq_groups_show(struct seq_file *seq, void *v) " 2^0 2^1 2^2 2^3 2^4 2^5 2^6 " " 2^7 2^8 2^9 2^10 2^11 2^12 2^13 ]\n"); - i = (sb->s_blocksize_bits + 2) * sizeof(sg.info.bb_counters[0]) + - sizeof(struct ext4_group_info); grinfo = ext4_get_group_info(sb, group); /* Load the group info in memory only if not already loaded. */ if (unlikely(EXT4_MB_GRP_NEED_INIT(grinfo))) { @@ -2319,7 +2320,7 @@ static int ext4_mb_seq_groups_show(struct seq_file *seq, void *v) buddy_loaded = 1; } - memcpy(&sg, ext4_get_group_info(sb, group), i); + memcpy(&sg, ext4_get_group_info(sb, group), sizeof(sg)); if (buddy_loaded) ext4_mb_unload_buddy(&e4b); @@ -2327,7 +2328,7 @@ static int ext4_mb_seq_groups_show(struct seq_file *seq, void *v) seq_printf(seq, "#%-5u: %-5u %-5u %-5u [", group, sg.info.bb_free, sg.info.bb_fragments, sg.info.bb_first_free); for (i = 0; i <= 13; i++) - seq_printf(seq, " %-5u", i <= sb->s_blocksize_bits + 1 ? + seq_printf(seq, " %-5u", i <= blocksize_bits + 1 ? sg.info.bb_counters[i] : 0); seq_printf(seq, " ]\n");