linux-kernel - Re: [PATCH 09/32] ext4: extended attribute value size limit is enforced by vfs

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]

Message-Id: <B0650C7F-0982-4D10-ADC9-925B376D92BE@dilger.ca>
Date:   Thu, 22 Jun 2017 13:40:25 -0600
From:   Andreas Dilger <adilger@...ger.ca>
To:     "Darrick J. Wong" <darrick.wong@...cle.com>
Cc:     Tahsin Erdogan <tahsin@...gle.com>, Jan Kara <jack@...e.cz>,
        Theodore Ts'o <tytso@....edu>, linux-ext4@...r.kernel.org,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH 09/32] ext4: extended attribute value size limit is
 enforced by vfs

On Jun 22, 2017, at 12:02 PM, Darrick J. Wong <darrick.wong@...cle.com> wrote:
> 
> On Wed, Jun 21, 2017 at 02:21:19PM -0700, Tahsin Erdogan wrote:
>> EXT4_XATTR_MAX_LARGE_EA_SIZE definition in ext4 is currently unused.
>> Besides, vfs enforces its own 64k limit which makes the 1MB limit in
>> ext4 redundant. Remove it.
> 
> Just FYI I believe the 64k VFS limit exists because XFS is the only fs
> to allow large xattr values, and its maximum is 64k (on-disk field size
> limitation).
> 
> I don't know if anyone actually wants larger values?  Now could be a
> time to have such a conversation, if anyone is so interested.

The EXT4_XATTR_MAX_LARGE_EA_SIZE limit of 1MB was also totally arbitrary,
but a reasonable upper limit for the atomic get/set interface used by
xattrs.  The underlying disk format could actually store xattrs of any size.

I'd think if we want to get huge xattrs that they should be handled by
having separate streams (e.g. open file descriptor, ioctl/syscall to select
a different stream number on that file) so that the data doesn't have to be
completely rewritten any time it is modified, but streams are frowned upon
by many Linux developers for security reasons so will probably be a no-go.

Cheers, Andreas

> --D
> 
>> 
>> Signed-off-by: Tahsin Erdogan <tahsin@...gle.com>
>> ---
>> fs/ext4/ext4.h | 6 ------
>> 1 file changed, 6 deletions(-)
>> 
>> diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h
>> index 5d5fc0d0e2bc..2cdd6070e348 100644
>> --- a/fs/ext4/ext4.h
>> +++ b/fs/ext4/ext4.h
>> @@ -2220,12 +2220,6 @@ struct mmpd_data {
>>  */
>> #define EXT4_MMP_MAX_CHECK_INTERVAL	300UL
>> 
>> -/*
>> - * Maximum size of xattr attributes for FEATURE_INCOMPAT_EA_INODE 1Mb
>> - * This limit is arbitrary, but is reasonable for the xattr API.
>> - */
>> -#define EXT4_XATTR_MAX_LARGE_EA_SIZE    (1024 * 1024)
>> -
>> /*
>>  * Function prototypes
>>  */
>> --
>> 2.13.1.611.g7e3b11ae1-goog
>> 


Cheers, Andreas






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