From patchwork Tue Feb 9 21:41:50 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Andrew Morton X-Patchwork-Id: 379990 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-15.8 required=3.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, HEADER_FROM_DIFFERENT_DOMAINS, INCLUDES_CR_TRAILER, INCLUDES_PATCH, MAILING_LIST_MULTI, SPF_HELO_NONE, SPF_PASS, URIBL_BLOCKED autolearn=unavailable autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 5046BC433E0 for ; Tue, 9 Feb 2021 23:33:57 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 1633C601FF for ; Tue, 9 Feb 2021 23:33:57 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S234268AbhBIXcX (ORCPT ); Tue, 9 Feb 2021 18:32:23 -0500 Received: from mail.kernel.org ([198.145.29.99]:55568 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S234039AbhBIWR2 (ORCPT ); Tue, 9 Feb 2021 17:17:28 -0500 Received: by mail.kernel.org (Postfix) with ESMTPSA id C6DD364EC4; Tue, 9 Feb 2021 21:41:50 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=linux-foundation.org; s=korg; t=1612906911; bh=LYrdgh9B9MJd+gEQVz2818OkEWazwLzDnyWKnJ/SkJc=; h=Date:From:To:Subject:In-Reply-To:From; b=NwBdFMqBo8f9en5/27ItLhAfFUmTW+kU2EPj6iLdVtMsfVl54rIDY69etgrwysTD8 RiFxT3pUeFIofS0/rpGy5WEs84NutBOHO+FPDwfgKs7wTEQYfihUWr854hp45Bytk8 kYzW1jd2geGqLD3+TXtCp6+mWBhSshlrvDaFStEI= Date: Tue, 09 Feb 2021 13:41:50 -0800 From: Andrew Morton To: akpm@linux-foundation.org, linux-mm@kvack.org, mm-commits@vger.kernel.org, phillip@squashfs.org.uk, pliard@google.com, stable@vger.kernel.org, torvalds@linux-foundation.org Subject: [patch 01/14] squashfs: avoid out of bounds writes in decompressors Message-ID: <20210209214150._Vu1-fOx4%akpm@linux-foundation.org> In-Reply-To: <20210209134115.4d933d446165cd0ed8977b03@linux-foundation.org> User-Agent: s-nail v14.8.16 Precedence: bulk List-ID: X-Mailing-List: stable@vger.kernel.org From: Phillip Lougher Subject: squashfs: avoid out of bounds writes in decompressors Patch series "Squashfs: fix BIO migration regression and add sanity checks". Patch [1/4] fixes a regression introduced by the "migrate from ll_rw_block usage to BIO" patch, which has produced a number of Sysbot/Syzkaller reports. Patches [2/4], [3/4], and [4/4] fix a number of filesystem corruption issues which have produced Sysbot reports in the id, inode and xattr lookup code. Each patch has been tested against the Sysbot reproducers using the given kernel configuration. They have the appropriate "Reported-by:" lines added. Additionally, all of the reproducer filesystems are indirectly fixed by patch [4/4] due to the fact they all have xattr corruption which is now detected there. Additional testing with other configurations and architectures (32bit, big endian), and normal filesystems has also been done to trap any inadvertent regressions caused by the additional sanity checks. This patch (of 4): This is a regression introduced by the patch "migrate from ll_rw_block usage to BIO". Sysbot/Syskaller has reported a number of "out of bounds writes" and "unable to handle kernel paging request in squashfs_decompress" errors which have been identified as a regression introduced by the above patch. Specifically, the patch removed the following sanity check if (length < 0 || length > output->length || (index + length) > msblk->bytes_used) This check did two things: 1. It ensured any reads were not beyond the end of the filesystem 2. It ensured that the "length" field read from the filesystem was within the expected maximum length. Without this any corrupted values can over-run allocated buffers. Link: https://lkml.kernel.org/r/20210204130249.4495-1-phillip@squashfs.org.uk Link: https://lkml.kernel.org/r/20210204130249.4495-2-phillip@squashfs.org.uk Fixes: 93e72b3c612adc ("squashfs: migrate from ll_rw_block usage to BIO") Reported-by: syzbot+6fba78f99b9afd4b5634@syzkaller.appspotmail.com Signed-off-by: Phillip Lougher Cc: Philippe Liard Cc: Signed-off-by: Andrew Morton --- fs/squashfs/block.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) --- a/fs/squashfs/block.c~squashfs-avoid-out-of-bounds-writes-in-decompressors +++ a/fs/squashfs/block.c @@ -196,9 +196,15 @@ int squashfs_read_data(struct super_bloc length = SQUASHFS_COMPRESSED_SIZE(length); index += 2; - TRACE("Block @ 0x%llx, %scompressed size %d\n", index, + TRACE("Block @ 0x%llx, %scompressed size %d\n", index - 2, compressed ? "" : "un", length); } + if (length < 0 || length > output->length || + (index + length) > msblk->bytes_used) { + res = -EIO; + goto out; + } + if (next_index) *next_index = index + length; From patchwork Tue Feb 9 21:41:53 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Andrew Morton X-Patchwork-Id: 379991 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-15.8 required=3.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, HEADER_FROM_DIFFERENT_DOMAINS, INCLUDES_CR_TRAILER, INCLUDES_PATCH, MAILING_LIST_MULTI, SPF_HELO_NONE, SPF_PASS, URIBL_BLOCKED autolearn=unavailable autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id EC37EC433E0 for ; Tue, 9 Feb 2021 23:32:16 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id C66DB64E3F for ; Tue, 9 Feb 2021 23:32:16 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S234142AbhBIXbt (ORCPT ); Tue, 9 Feb 2021 18:31:49 -0500 Received: from mail.kernel.org ([198.145.29.99]:55566 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S234155AbhBIWO7 (ORCPT ); Tue, 9 Feb 2021 17:14:59 -0500 Received: by mail.kernel.org (Postfix) with ESMTPSA id 2EAD164EC5; Tue, 9 Feb 2021 21:41:54 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=linux-foundation.org; s=korg; t=1612906914; bh=+R4mdfV/B/rzJsOhjEYGlGH54+uq4W9gWlFo2bOy89Y=; h=Date:From:To:Subject:In-Reply-To:From; b=f3PtiMNIPro9hXox81mPiP3e4D527hQeznEND+xoU/v7qx4W1TuZ/OnIYCKTwrYAq PpAMRFDeRDmIH8NpbTbpDHyaPe2vBH/AnXL0XL5d0xGfeUKJhEuqnszeMkxTgmjxy2 +w0sQ945lF2IBuDnQhU1ghv8ZDXStkkapQg2uQ68= Date: Tue, 09 Feb 2021 13:41:53 -0800 From: Andrew Morton To: akpm@linux-foundation.org, linux-mm@kvack.org, mm-commits@vger.kernel.org, phillip@squashfs.org.uk, stable@vger.kernel.org, torvalds@linux-foundation.org Subject: [patch 02/14] squashfs: add more sanity checks in id lookup Message-ID: <20210209214153.l0Cs-q9SO%akpm@linux-foundation.org> In-Reply-To: <20210209134115.4d933d446165cd0ed8977b03@linux-foundation.org> User-Agent: s-nail v14.8.16 Precedence: bulk List-ID: X-Mailing-List: stable@vger.kernel.org From: Phillip Lougher Subject: squashfs: add more sanity checks in id lookup Sysbot has reported a number of "slab-out-of-bounds reads" and "use-after-free read" errors which has been identified as being caused by a corrupted index value read from the inode. This could be because the metadata block is uncompressed, or because the "compression" bit has been corrupted (turning a compressed block into an uncompressed block). This patch adds additional sanity checks to detect this, and the following corruption. 1. It checks against corruption of the ids count. This can either lead to a larger table to be read, or a smaller than expected table to be read. In the case of a too large ids count, this would often have been trapped by the existing sanity checks, but this patch introduces a more exact check, which can identify too small values. 2. It checks the contents of the index table for corruption. Link: https://lkml.kernel.org/r/20210204130249.4495-3-phillip@squashfs.org.uk Signed-off-by: Phillip Lougher Reported-by: syzbot+b06d57ba83f604522af2@syzkaller.appspotmail.com Reported-by: syzbot+c021ba012da41ee9807c@syzkaller.appspotmail.com Reported-by: syzbot+5024636e8b5fd19f0f19@syzkaller.appspotmail.com Reported-by: syzbot+bcbc661df46657d0fa4f@syzkaller.appspotmail.com Cc: Signed-off-by: Andrew Morton --- fs/squashfs/id.c | 40 ++++++++++++++++++++++++++------- fs/squashfs/squashfs_fs_sb.h | 1 fs/squashfs/super.c | 6 ++-- fs/squashfs/xattr.h | 10 +++++++- 4 files changed, 45 insertions(+), 12 deletions(-) --- a/fs/squashfs/id.c~squashfs-add-more-sanity-checks-in-id-lookup +++ a/fs/squashfs/id.c @@ -35,10 +35,15 @@ int squashfs_get_id(struct super_block * struct squashfs_sb_info *msblk = sb->s_fs_info; int block = SQUASHFS_ID_BLOCK(index); int offset = SQUASHFS_ID_BLOCK_OFFSET(index); - u64 start_block = le64_to_cpu(msblk->id_table[block]); + u64 start_block; __le32 disk_id; int err; + if (index >= msblk->ids) + return -EINVAL; + + start_block = le64_to_cpu(msblk->id_table[block]); + err = squashfs_read_metadata(sb, &disk_id, &start_block, &offset, sizeof(disk_id)); if (err < 0) @@ -56,7 +61,10 @@ __le64 *squashfs_read_id_index_table(str u64 id_table_start, u64 next_table, unsigned short no_ids) { unsigned int length = SQUASHFS_ID_BLOCK_BYTES(no_ids); + unsigned int indexes = SQUASHFS_ID_BLOCKS(no_ids); + int n; __le64 *table; + u64 start, end; TRACE("In read_id_index_table, length %d\n", length); @@ -67,20 +75,36 @@ __le64 *squashfs_read_id_index_table(str return ERR_PTR(-EINVAL); /* - * length bytes should not extend into the next table - this check - * also traps instances where id_table_start is incorrectly larger - * than the next table start + * The computed size of the index table (length bytes) should exactly + * match the table start and end points */ - if (id_table_start + length > next_table) + if (length != (next_table - id_table_start)) return ERR_PTR(-EINVAL); table = squashfs_read_table(sb, id_table_start, length); + if (IS_ERR(table)) + return table; /* - * table[0] points to the first id lookup table metadata block, this - * should be less than id_table_start + * table[0], table[1], ... table[indexes - 1] store the locations + * of the compressed id blocks. Each entry should be less than + * the next (i.e. table[0] < table[1]), and the difference between them + * should be SQUASHFS_METADATA_SIZE or less. table[indexes - 1] + * should be less than id_table_start, and again the difference + * should be SQUASHFS_METADATA_SIZE or less */ - if (!IS_ERR(table) && le64_to_cpu(table[0]) >= id_table_start) { + for (n = 0; n < (indexes - 1); n++) { + start = le64_to_cpu(table[n]); + end = le64_to_cpu(table[n + 1]); + + if (start >= end || (end - start) > SQUASHFS_METADATA_SIZE) { + kfree(table); + return ERR_PTR(-EINVAL); + } + } + + start = le64_to_cpu(table[indexes - 1]); + if (start >= id_table_start || (id_table_start - start) > SQUASHFS_METADATA_SIZE) { kfree(table); return ERR_PTR(-EINVAL); } --- a/fs/squashfs/squashfs_fs_sb.h~squashfs-add-more-sanity-checks-in-id-lookup +++ a/fs/squashfs/squashfs_fs_sb.h @@ -64,5 +64,6 @@ struct squashfs_sb_info { unsigned int inodes; unsigned int fragments; int xattr_ids; + unsigned int ids; }; #endif --- a/fs/squashfs/super.c~squashfs-add-more-sanity-checks-in-id-lookup +++ a/fs/squashfs/super.c @@ -166,6 +166,7 @@ static int squashfs_fill_super(struct su msblk->directory_table = le64_to_cpu(sblk->directory_table_start); msblk->inodes = le32_to_cpu(sblk->inodes); msblk->fragments = le32_to_cpu(sblk->fragments); + msblk->ids = le16_to_cpu(sblk->no_ids); flags = le16_to_cpu(sblk->flags); TRACE("Found valid superblock on %pg\n", sb->s_bdev); @@ -177,7 +178,7 @@ static int squashfs_fill_super(struct su TRACE("Block size %d\n", msblk->block_size); TRACE("Number of inodes %d\n", msblk->inodes); TRACE("Number of fragments %d\n", msblk->fragments); - TRACE("Number of ids %d\n", le16_to_cpu(sblk->no_ids)); + TRACE("Number of ids %d\n", msblk->ids); TRACE("sblk->inode_table_start %llx\n", msblk->inode_table); TRACE("sblk->directory_table_start %llx\n", msblk->directory_table); TRACE("sblk->fragment_table_start %llx\n", @@ -236,8 +237,7 @@ static int squashfs_fill_super(struct su allocate_id_index_table: /* Allocate and read id index table */ msblk->id_table = squashfs_read_id_index_table(sb, - le64_to_cpu(sblk->id_table_start), next_table, - le16_to_cpu(sblk->no_ids)); + le64_to_cpu(sblk->id_table_start), next_table, msblk->ids); if (IS_ERR(msblk->id_table)) { errorf(fc, "unable to read id index table"); err = PTR_ERR(msblk->id_table); --- a/fs/squashfs/xattr.h~squashfs-add-more-sanity-checks-in-id-lookup +++ a/fs/squashfs/xattr.h @@ -17,8 +17,16 @@ extern int squashfs_xattr_lookup(struct static inline __le64 *squashfs_read_xattr_id_table(struct super_block *sb, u64 start, u64 *xattr_table_start, int *xattr_ids) { + struct squashfs_xattr_id_table *id_table; + + id_table = squashfs_read_table(sb, start, sizeof(*id_table)); + if (IS_ERR(id_table)) + return (__le64 *) id_table; + + *xattr_table_start = le64_to_cpu(id_table->xattr_table_start); + kfree(id_table); + ERROR("Xattrs in filesystem, these will be ignored\n"); - *xattr_table_start = start; return ERR_PTR(-ENOTSUPP); } From patchwork Tue Feb 9 21:42:14 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Andrew Morton X-Patchwork-Id: 379992 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-15.8 required=3.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, HEADER_FROM_DIFFERENT_DOMAINS, INCLUDES_CR_TRAILER, INCLUDES_PATCH, MAILING_LIST_MULTI, SPF_HELO_NONE, SPF_PASS, URIBL_BLOCKED autolearn=unavailable autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id B38CCC433E0 for ; Tue, 9 Feb 2021 22:41:34 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 6F50164E2E for ; Tue, 9 Feb 2021 22:41:34 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S233090AbhBIWkm (ORCPT ); Tue, 9 Feb 2021 17:40:42 -0500 Received: from mail.kernel.org ([198.145.29.99]:55934 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S234216AbhBIWRy (ORCPT ); Tue, 9 Feb 2021 17:17:54 -0500 Received: by mail.kernel.org (Postfix) with ESMTPSA id 92EB764ECF; Tue, 9 Feb 2021 21:42:14 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=linux-foundation.org; s=korg; t=1612906935; bh=Z3tdZ8g+QGli3UaRB37iBSBQLEU+YRh7a0FWPInUBM8=; h=Date:From:To:Subject:In-Reply-To:From; b=SWYKD6NGlC9KMNLk++hTqYt31hZxGQv6gboga0c39AkG5sfqJECfOj2r0GKn3Ze0y I32vPVIow0SsyJyVNqJ8TYyndmVya4dNB1hN+628ipc6MdtEy0lGwaLDcPeIfoEW0M ZOnvatC652mK98beEw2aIVxU2C6Ei7vOxpktsnMg= Date: Tue, 09 Feb 2021 13:42:14 -0800 From: Andrew Morton To: akpm@linux-foundation.org, amir73il@gmail.com, borntraeger@de.ibm.com, chris@chrisdown.name, gor@linux.ibm.com, hca@linux.ibm.com, hughd@google.com, linux-mm@kvack.org, mm-commits@vger.kernel.org, seth.forshee@canonical.com, stable@vger.kernel.org, torvalds@linux-foundation.org Subject: [patch 08/14] tmpfs: disallow CONFIG_TMPFS_INODE64 on s390 Message-ID: <20210209214214.Eh4EHTHRW%akpm@linux-foundation.org> In-Reply-To: <20210209134115.4d933d446165cd0ed8977b03@linux-foundation.org> User-Agent: s-nail v14.8.16 Precedence: bulk List-ID: X-Mailing-List: stable@vger.kernel.org From: Seth Forshee Subject: tmpfs: disallow CONFIG_TMPFS_INODE64 on s390 Currently there is an assumption in tmpfs that 64-bit architectures also have a 64-bit ino_t. This is not true on s390 which has a 32-bit ino_t. With CONFIG_TMPFS_INODE64=y tmpfs mounts will get 64-bit inode numbers and display "inode64" in the mount options, but passing the "inode64" mount option will fail. This leads to the following behavior: # mkdir mnt # mount -t tmpfs nodev mnt # mount -o remount,rw mnt mount: /home/ubuntu/mnt: mount point not mounted or bad option. As mount sees "inode64" in the mount options and thus passes it in the options for the remount. So prevent CONFIG_TMPFS_INODE64 from being selected on s390. Link: https://lkml.kernel.org/r/20210205230620.518245-1-seth.forshee@canonical.com Fixes: ea3271f7196c ("tmpfs: support 64-bit inums per-sb") Signed-off-by: Seth Forshee Acked-by: Hugh Dickins Cc: Chris Down Cc: Hugh Dickins Cc: Amir Goldstein Cc: Heiko Carstens Cc: Vasily Gorbik Cc: Christian Borntraeger Cc: [5.9+] Signed-off-by: Andrew Morton --- fs/Kconfig | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) --- a/fs/Kconfig~tmpfs-disallow-config_tmpfs_inode64-on-s390 +++ a/fs/Kconfig @@ -203,7 +203,7 @@ config TMPFS_XATTR config TMPFS_INODE64 bool "Use 64-bit ino_t by default in tmpfs" - depends on TMPFS && 64BIT + depends on TMPFS && 64BIT && !S390 default n help tmpfs has historically used only inode numbers as wide as an unsigned From patchwork Tue Feb 9 21:42:32 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Andrew Morton X-Patchwork-Id: 379989 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-15.8 required=3.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, HEADER_FROM_DIFFERENT_DOMAINS, INCLUDES_CR_TRAILER, INCLUDES_PATCH, MAILING_LIST_MULTI, SPF_HELO_NONE, SPF_PASS, URIBL_BLOCKED autolearn=unavailable autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id D6F76C43381 for ; Wed, 10 Feb 2021 00:36:25 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id AAE0164E56 for ; Wed, 10 Feb 2021 00:36:25 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S233310AbhBJAgD (ORCPT ); Tue, 9 Feb 2021 19:36:03 -0500 Received: from mail.kernel.org ([198.145.29.99]:34458 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S233916AbhBJAWD (ORCPT ); Tue, 9 Feb 2021 19:22:03 -0500 Received: by mail.kernel.org (Postfix) with ESMTPSA id AF23F64EDA; Tue, 9 Feb 2021 21:42:32 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=linux-foundation.org; s=korg; t=1612906953; bh=kHnDG8x4GyiYVin0rB8XvMSbUslfYk56UCOET8LheBY=; h=Date:From:To:Subject:In-Reply-To:From; b=XuAPpHlnBbDLdThvy6LVTXtXwpHSliCwB5gHS2fCVa9EpXeBPARhiCZiialw6fX+y tYtQekoyuV0FEa3w4riveF9RVrV+cnf8gqop6YiXZ7lWM+DhlK7inU9NT+/YUVhQ5h uftN15xzHoOUUhyj7WJWrUOcrIjHeU7xNubX/De0= Date: Tue, 09 Feb 2021 13:42:32 -0800 From: Andrew Morton To: akpm@linux-foundation.org, aneesh.kumar@linux.ibm.com, bharata@linux.ibm.com, catalin.marinas@arm.com, cl@linux.com, guro@fb.com, hannes@cmpxchg.org, iamjoonsoo.kim@lge.com, jannh@google.com, linux-mm@kvack.org, mgorman@techsingularity.net, mhocko@kernel.org, mm-commits@vger.kernel.org, rientjes@google.com, shakeelb@google.com, stable@vger.kernel.org, torvalds@linux-foundation.org, vbabka@suse.cz, vincent.guittot@linaro.org, will@kernel.org Subject: [patch 13/14] mm, slub: better heuristic for number of cpus when calculating slab order Message-ID: <20210209214232.hlVJaEmRu%akpm@linux-foundation.org> In-Reply-To: <20210209134115.4d933d446165cd0ed8977b03@linux-foundation.org> User-Agent: s-nail v14.8.16 Precedence: bulk List-ID: X-Mailing-List: stable@vger.kernel.org From: Vlastimil Babka Subject: mm, slub: better heuristic for number of cpus when calculating slab order When creating a new kmem cache, SLUB determines how large the slab pages will based on number of inputs, including the number of CPUs in the system. Larger slab pages mean that more objects can be allocated/free from per-cpu slabs before accessing shared structures, but also potentially more memory can be wasted due to low slab usage and fragmentation. The rough idea of using number of CPUs is that larger systems will be more likely to benefit from reduced contention, and also should have enough memory to spare. Number of CPUs used to be determined as nr_cpu_ids, which is number of possible cpus, but on some systems many will never be onlined, thus commit 045ab8c9487b ("mm/slub: let number of online CPUs determine the slub page order") changed it to nr_online_cpus(). However, for kmem caches created early before CPUs are onlined, this may lead to permamently low slab page sizes. Vincent reports a regression [1] of hackbench on arm64 systems: > I'm facing significant performances regression on a large arm64 server > system (224 CPUs). Regressions is also present on small arm64 system > (8 CPUs) but in a far smaller order of magnitude > On 224 CPUs system : 9 iterations of hackbench -l 16000 -g 16 > v5.11-rc4 : 9.135sec (+/- 0.45%) > v5.11-rc4 + revert this patch: 3.173sec (+/- 0.48%) > v5.10: 3.136sec (+/- 0.40%) Mel reports a regression [2] of hackbench on x86_64, with lockstat suggesting page allocator contention: > i.e. the patch incurs a 7% to 32% performance penalty. This bisected > cleanly yesterday when I was looking for the regression and then found > the thread. > Numerous caches change size. For example, kmalloc-512 goes from order-0 > (vanilla) to order-2 with the revert. > So mostly this is down to the number of times SLUB calls into the page > allocator which only caches order-0 pages on a per-cpu basis. Clearly num_online_cpus() doesn't work too early in bootup. We could change the order dynamically in a memory hotplug callback, but runtime order changing for existing kmem caches has been already shown as dangerous, and removed in 32a6f409b693 ("mm, slub: remove runtime allocation order changes"). It could be resurrected in a safe manner with some effort, but to fix the regression we need something simpler. We could use num_present_cpus() that should be the number of physically present CPUs even before they are onlined. That would work for PowerPC [3], which triggered the original commit, but that still doesn't work on arm64 [4] as explained in [5]. So this patch tries to determine the best available value without specific arch knowledge. - num_present_cpus() if the number is larger than 1, as that means the arch is likely setting it properly - nr_cpu_ids otherwise This should fix the reported regressions while also keeping the effect of 045ab8c9487b for PowerPC systems. It's possible there are configurations where num_present_cpus() is 1 during boot while nr_cpu_ids is at the same time bloated, so these (if they exist) would keep the large orders based on nr_cpu_ids as was before 045ab8c9487b. [1] https://lore.kernel.org/linux-mm/CAKfTPtA_JgMf_+zdFbcb_V9rM7JBWNPjAz9irgwFj7Rou=xzZg@mail.gmail.com/ [2] https://lore.kernel.org/linux-mm/20210128134512.GF3592@techsingularity.net/ [3] https://lore.kernel.org/linux-mm/20210123051607.GC2587010@in.ibm.com/ [4] https://lore.kernel.org/linux-mm/CAKfTPtAjyVmS5VYvU6DBxg4-JEo5bdmWbngf-03YsY18cmWv_g@mail.gmail.com/ [5] https://lore.kernel.org/linux-mm/20210126230305.GD30941@willie-the-truck/ Link: https://lkml.kernel.org/r/20210208134108.22286-1-vbabka@suse.cz Fixes: 045ab8c9487b ("mm/slub: let number of online CPUs determine the slub page order") Signed-off-by: Vlastimil Babka Reported-by: Vincent Guittot Reported-by: Mel Gorman Tested-by: Vincent Guittot Cc: Catalin Marinas Cc: Aneesh Kumar K.V Cc: Bharata B Rao Cc: Christoph Lameter Cc: Roman Gushchin Cc: Johannes Weiner Cc: Joonsoo Kim Cc: Jann Horn Cc: Michal Hocko Cc: David Rientjes Cc: Shakeel Butt Cc: Will Deacon Cc: Signed-off-by: Andrew Morton --- mm/slub.c | 18 ++++++++++++++++-- 1 file changed, 16 insertions(+), 2 deletions(-) --- a/mm/slub.c~mm-slub-better-heuristic-for-number-of-cpus-when-calculating-slab-order +++ a/mm/slub.c @@ -3423,6 +3423,7 @@ static inline int calculate_order(unsign unsigned int order; unsigned int min_objects; unsigned int max_objects; + unsigned int nr_cpus; /* * Attempt to find best configuration for a slab. This @@ -3433,8 +3434,21 @@ static inline int calculate_order(unsign * we reduce the minimum objects required in a slab. */ min_objects = slub_min_objects; - if (!min_objects) - min_objects = 4 * (fls(num_online_cpus()) + 1); + if (!min_objects) { + /* + * Some architectures will only update present cpus when + * onlining them, so don't trust the number if it's just 1. But + * we also don't want to use nr_cpu_ids always, as on some other + * architectures, there can be many possible cpus, but never + * onlined. Here we compromise between trying to avoid too high + * order on systems that appear larger than they are, and too + * low order on systems that appear smaller than they are. + */ + nr_cpus = num_present_cpus(); + if (nr_cpus <= 1) + nr_cpus = nr_cpu_ids; + min_objects = 4 * (fls(nr_cpus) + 1); + } max_objects = order_objects(slub_max_order, size); min_objects = min(min_objects, max_objects);