lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
Open Source and information security mailing list archives
| ||
|
Message-ID: <877cpx5rl5.fsf@suse.de> Date: Mon, 14 Aug 2023 15:52:54 -0400 From: Gabriel Krisman Bertazi <krisman@...e.de> To: Eric Biggers <ebiggers@...nel.org> Cc: linux-ext4@...r.kernel.org, Theodore Ts'o <tytso@....edu>, linux-fsdevel@...r.kernel.org, linux-f2fs-devel@...ts.sourceforge.net Subject: Re: [PATCH 1/3] ext4: reject casefold inode flag without casefold feature Eric Biggers <ebiggers@...nel.org> writes: > On Mon, Aug 14, 2023 at 03:09:33PM -0400, Gabriel Krisman Bertazi wrote: >> Eric Biggers <ebiggers@...nel.org> writes: >> >> > From: Eric Biggers <ebiggers@...gle.com> >> > >> > It is invalid for the casefold inode flag to be set without the casefold >> > superblock feature flag also being set. e2fsck already considers this >> > case to be invalid and handles it by offering to clear the casefold flag >> > on the inode. __ext4_iget() also already considered this to be invalid, >> > sort of, but it only got so far as logging an error message; it didn't >> > actually reject the inode. Make it reject the inode so that other code >> > doesn't have to handle this case. This matches what f2fs does. >> > >> > Note: we could check 's_encoding != NULL' instead of >> > ext4_has_feature_casefold(). This would make the check robust against >> > the casefold feature being enabled by userspace writing to the page >> > cache of the mounted block device. However, it's unsolvable in general >> > for filesystems to be robust against concurrent writes to the page cache >> > of the mounted block device. Though this very particular scenario >> > involving the casefold feature is solvable, we should not pretend that >> > we can support this model, so let's just check the casefold feature. >> > tune2fs already forbids enabling casefold on a mounted filesystem. >> >> just because we can't fix the general issue for the entire filesystem >> doesn't mean this case *must not* ever be addressed. What is the >> advantage of making the code less robust against the syzbot code? Just >> check sb->s_encoding and be safe later knowing the unicode map is >> available. >> > > Just to make sure, it sounds like you agree that the late checks of ->s_encoding > are not needed and only __ext4_iget() should handle it, right? That simplifies > the code so it is obviously beneficial if we can do it. Yes. After we get the inode from __ext4_iget, I think it doesn't matter if the user went behind our back straight to the block device and changed the superblock to remove the feature bit. If we already loaded ->s_encoding, it won't be unloaded, so only checking at ext4_iget should be enough, as far as I can tell. -- Gabriel Krisman Bertazi
Powered by blists - more mailing lists