Received-SPF: Pass (protection.outlook.com: domain of mit.edu designates
 18.9.28.11 as permitted sender) receiver=protection.outlook.com;
 client-ip=18.9.28.11; helo=outgoing.mit.edu;
Date:   Mon, 11 Feb 2019 01:01:27 -0500
From:   "Theodore Y. Ts'o" <tytso@mit.edu>
To:     yangerkun <yangerkun@huawei.com>
CC:     <jack@suse.com>, <miaoxie@huawei.com>, <yi.zhang@huawei.com>,
        <houtao1@huawei.com>, <linux-ext4@vger.kernel.org>
Subject: Re: [PATCH V2 4/4] ext4: add mask of ext4 flags to swap
Message-ID: <20190211060127.GM23000@mit.edu>
References: <20190122065823.67957-1-yangerkun@huawei.com>
 <20190122065823.67957-5-yangerkun@huawei.com>
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Disposition: inline
In-Reply-To: <20190122065823.67957-5-yangerkun@huawei.com>
User-Agent: Mutt/1.10.1 (2018-07-13)
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 11 Feb 2019 06:01:29.6395
 (UTC)
X-MS-Exchange-CrossTenant-Network-Message-Id: ddf1e479-a61f-439b-0b45-08d68fe65e1e
X-MS-Exchange-CrossTenant-Id: 64afd9ba-0ecf-4acf-bc36-935f6235ba8b
X-MS-Exchange-CrossTenant-OriginalAttributedTenantConnectingIp: TenantId=64afd9ba-0ecf-4acf-bc36-935f6235ba8b;Ip=[18.9.28.11];Helo=[outgoing.mit.edu]
X-MS-Exchange-CrossTenant-FromEntityHeader: HybridOnPrem
X-MS-Exchange-Transport-CrossTenantHeadersStamped: SN6PR01MB3757
Sender: linux-ext4-owner@vger.kernel.org
Precedence: bulk
List-ID: <linux-ext4.vger.kernel.org>
X-Mailing-List: linux-ext4@vger.kernel.org

Thanks, applied.

I've simplified the commit description, and also dropped the
compression related flags (which are not currently being used at all)
from the list of flags that should be swapped.

One of the reasons for simplifying the commit description is if the
goal is to make the EXT4_IOC_SWAP_BOOT proof against hostile/malicious
userspace, then we shouldn't allow inodes that have the
EXT4_JOURNAL_DATA_FL set from being used a source for the bootloader
inode.

That's because if inode A has the JOURNAL_DATA flag set and a block B
belonging to inode A is freshly written to it (and thus journalled),
and then we swap it into the bootloader inode, and then swap it back
out with inode C, which does not have the JOURNAL_DATA flag, and then
that inode is immediately deleted, we won't write a revoke record for
block B into the journal.  If that block gets reused, and then we
crash (or the malicious root user deliberately crashes the system),
when the journal is replayed, we could end up corrupting block B.

This can be dealt with (see ext4_change_inode_journal_flag) but doing
so requires doing some drastic things, and the simpler solution is to
simply prohibit using inodes that have the JOURNAL_DATA flag set from
being used by EXT4_IOC_SWAP_BOOT, since there's no real use for
allowing case anyway.

This may be overkill, since you have to be root to use
EXT4_IOC_SWAP_BOOT, and a malcious root user who is determined to
screw up the system can find much more simpler ways to do so.  On the
other hand there are programs run as root written by clueless
developers, and systems which simulate such developers (e.g.,
syzkaller :-).  Previously we were a bit less careful with
EXT4_IOC_SWAP_BOOT because it could only be used by root, and only
very specialized programs would need to use it, and it was kind of
assumed that those root users and such specialized programs wouldn't
be *trying* to mess things up.

Anway, I'll follow up your patch series with a patch which prohibits
files that have EXT4_JOURNAL_DATA_FL set from being used with
EXT4_IOC_SWAP_BOOT.

Cheers,

					- Ted