linux-ext4 - Re: [PATCH] ext4: fix reading new encrypted symlinks on no-journal filesystems

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Fri, 18 Nov 2016 14:52:22 -0700
From:   Andreas Dilger <adilger@...ger.ca>
To:     Eric Biggers <ebiggers@...gle.com>
Cc:     linux-ext4 <linux-ext4@...r.kernel.org>,
        Theodore Ts'o <tytso@....edu>
Subject: Re: [PATCH] ext4: fix reading new encrypted symlinks on no-journal filesystems


> On Nov 18, 2016, at 11:47 AM, Eric Biggers <ebiggers@...gle.com> wrote:
> 
> On Thu, Nov 17, 2016 at 07:20:24PM -0700, Andreas Dilger wrote:
>> On Nov 16, 2016, at 10:50 AM, Eric Biggers <ebiggers@...gle.com> wrote:
>>> 
>>> On a filesystem with no journal, a symlink longer than about 32
>>> characters (exact length depending on padding for encryption) could not
>>> be followed or read immediately after being created in an encrypted
>>> directory.  This happened because when the symlink data went through the
>>> delayed allocation path instead of the journaling path, the symlink was
>>> incorrectly detected as a "fast" symlink rather than a "slow" symlink
>>> until its data was written out.
>> 
>> IMHO, this again exposes an issue that we've seen with "fast" vs. "slow"
>> symlink detection several times in the past whenever there is a data block
>> allocated for a fast symlink (e.g. when xattrs were allowed on symlinks).
>> 
>> int ext4_inode_is_fast_symlink(struct inode *inode)
>> {
>>        int ea_blocks = EXT4_I(inode)->i_file_acl ?
>>                EXT4_CLUSTER_SIZE(inode->i_sb) >> 9 : 0;
>> 
>>        if (ext4_has_inline_data(inode))
>>                return 0;
>> 
>>        return (S_ISLNK(inode->i_mode) && inode->i_blocks - ea_blocks == 0);
>> }
>> 
>> Instead of depending on the i_blocks count to detect slow symlinks, we
>> should just check the i_size < EXT4_N_BLOCKS * 4 (or <=, need to verify).
>> I believe this has always been true for fast symlinks, so it should be
>> OK to make this change.  That will isolate us from future changes that
>> may add block allocations to symlinks.
>> 
> 
> Yes, this would be a much nicer way to detect fast symlinks.
> 
> The only thing I'd be concerned about is the possibility of pre-existing
> "slow" symlinks that actually have targets short enough to be "fast"
> symlinks, perhaps in filesystems created by old drivers or by external
> tools.  If such links happened to work before, then a change to check
> i_size would break them.
> 
> This may not be an issue in practice.  I checked some old ext4 versions,
> ext2 from Linux 0.99.7, e2fsprogs, Android's ext4_utils, and FreeBSD's
> ext2 driver.
> They all create "fast" symlinks if the length of the symlink target length
> excluding the terminating null (i_size) is < 60.

I did a similar analysis with similar results.


Cheers, Andreas






Download attachment "signature.asc" of type "application/pgp-signature" (834 bytes)