linux-ext4 - Re: [PATCH] mke2fs: avoid inode number error with large FS

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: <d7b615e9-baf5-1998-13a2-f24c8cdebb6b@redhat.com>
Date:   Mon, 12 Feb 2018 10:23:53 -0600
From:   Eric Sandeen <esandeen@...hat.com>
To:     Lukas Czerner <lczerner@...hat.com>,
        Artem Blagodarenko <artem.blagodarenko@...il.com>
Cc:     linux-ext4@...r.kernel.org, adilger.kernel@...ger.ca,
        Alexey Lyashkov <alexey.lyashkov@...il.com>
Subject: Re: [PATCH] mke2fs: avoid inode number error with large FS

On 2/12/18 10:06 AM, Eric Sandeen wrote:
> 
> 
> On 2/12/18 9:45 AM, Lukas Czerner wrote:
>> On Mon, Feb 12, 2018 at 02:14:19PM +0300, Artem Blagodarenko wrote:
>>> From: Alexey Lyashkov <alexey.lyashkov@...il.com>
>>>
>>> Sometimes during system deployment customers are faced with system
>>> formating problem for given inodes/bytes rate. User need to recalucate
>>> this rate and start formating again.
>>>
>>> This patch adds code that limit inodes count instead of error return,
>>> to use all inodes in the filesystem.
>>
>> Hi,
>>
>> in this case then you do not have byte-per-inode ratio you've
>> specified. So why to specify it in the first place ?
>>
>> Maybe I am missing something but I would think that if you specify -i
>> then you know what you want and if it's not possible then I would not
>> expect the mke2fs to just succeed regardless. I guess it's confusing.
> 
> I agree that fixing up incorrect/impossible format specifications and
> continuing is not preferable; it really makes the behavior matrix complex
> when some incorrect options are fixed on the fly, while others fail.
> 
> And worse, this creates a new "default" behavior which comes into play
> only when specific incorrect mkfs options are explicitly provided.
> 
> When an admin stops using mkfs defaults and starts manually specifying
> geometry, the onus is on /them/ to specify options which are valid.

Ok, I have read the patch more carefully now ;)

So prior to this patch, on a filesystem /with/ the 64bit feature, we
already silently limit inode count to MAX_32_NUM.  So if an inode ratio
is specified which would be > MAX_32_NUM we silently cap it, but only
if 64bit is turned on.

With your patch, you change the behavior so that the MAX_32_NUM
cap/adjustment is always in place both with and without the 64bit
feature, but it only warns the user when the 64bit feature is not present?

So I guess there is precedent for the behavior.  I'd have preferred that
mkfs simply fail if an invalid inode ratio was specified, but I guess
that is already not the case.  So perhaps fixing it on the fly for
both 64bit and !64bit is reasonable, though I'm not sure why we should
warn with !64bit and be silent with 64bit...

I guess my preference would be to always fail the mkfs if an invalid
inode ratio was specified on the commandline, regardless of 64bit
feature presence.  Silent fix-ups of incorrect specifications tend
to violate the principle of least surprise.

-Eric