From mboxrd@z Thu Jan 1 00:00:00 1970 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=-7.0 required=3.0 tests=BAYES_00, HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS, USER_AGENT_GIT autolearn=no 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 D8DC1C433E0 for ; Tue, 4 Aug 2020 07:25:55 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id E7E6C2076E for ; Tue, 4 Aug 2020 07:25:55 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1728835AbgHDHZy (ORCPT ); Tue, 4 Aug 2020 03:25:54 -0400 Received: from mx2.suse.de ([195.135.220.15]:36216 "EHLO mx2.suse.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726808AbgHDHZy (ORCPT ); Tue, 4 Aug 2020 03:25:54 -0400 X-Virus-Scanned: by amavisd-new at test-mx.suse.de Received: from relay2.suse.de (unknown [195.135.221.27]) by mx2.suse.de (Postfix) with ESMTP id 687B5AC85 for ; Tue, 4 Aug 2020 07:26:09 +0000 (UTC) From: Qu Wenruo To: linux-btrfs@vger.kernel.org Subject: [PATCH v2 0/2] btrfs: remove the inode_need_compress() call in Date: Tue, 4 Aug 2020 15:25:46 +0800 Message-Id: <20200804072548.34001-1-wqu@suse.com> X-Mailer: git-send-email 2.28.0 MIME-Version: 1.0 Content-Transfer-Encoding: 8bit Sender: linux-btrfs-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-btrfs@vger.kernel.org This is an attempt to remove the inode_need_compress() call in compress_file_extent(). As that compress_file_extent() can race with inode ioctl or bad compression ratio, to cause NULL pointer dereferecen for @pages, it's nature to try to remove that inode_need_compress() to remove the race completely. However that's not that easy, we have the following problems: - We still need to check @pages anyway That @pages check is for kcalloc() failure, so what we really get is just removing one indent from the if (inode_need_compress()). Everything else is still the same (in fact, even worse, see below problems) - Behavior change Before that change, every async_chunk does their check on INODE_NO_COMPRESS flags. If we hit any bad compression ratio, all incoming async_chunk will fall back to plain text write. But if we remove that inode_need_compress() check, then we still try to compress, and lead to potentially wasted CPU times. - Still race between compression disable and NULL pointer dereferecen There is a hidden race, mostly exposed by btrfs/071 test case, that we have "compress_type = fs_info->compress_type", so we can still hit case where that compress_type is NONE (caused by remount -o nocompress), and then btrfs_compress_pages() will return -E2BIG, without modifying @nr_pages Then later when we cleanup @pages, we try to access pages[i]->mapping, triggering NULL pointer dereference. This will be address in the first patch though. Changelog: v2: - Fix a bad commit merge Which merges the commit message for the first patch, though the content is still correct. Qu Wenruo (2): btrfs: prevent NULL pointer dereference in compress_file_range() when btrfs_compress_pages() hits default case btrfs: inode: don't re-evaluate inode_need_compress() in compress_file_extent() fs/btrfs/compression.c | 10 ++-- fs/btrfs/inode.c | 106 ++++++++++++++++++++--------------------- 2 files changed, 59 insertions(+), 57 deletions(-) -- 2.28.0