linux-ext4 - Re: [PATCH 06/15] ext2fs: add new APIs needed for fast commits

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: <20201202184458.GJ390058@mit.edu>
Date:   Wed, 2 Dec 2020 13:44:58 -0500
From:   "Theodore Y. Ts'o" <tytso@....edu>
To:     Harshad Shirwadkar <harshadshirwadkar@...il.com>
Cc:     linux-ext4@...r.kernel.org
Subject: Re: [PATCH 06/15] ext2fs: add new APIs needed for fast commits

On Fri, Nov 20, 2020 at 11:15:57AM -0800, Harshad Shirwadkar wrote:
> This patch adds the following new APIs:
> 
> Count the total number of blocks occupied by inode including
> intermediate extent tree nodes.
> extern blk64_t ext2fs_count_blocks(ext2_filsys fs, ext2_ino_t ino,
>                                        struct ext2_inode *inode);
> 
> Convert ext3_extent to ext2fs_extent.
> extern void ext2fs_convert_extent(struct ext2fs_extent *to,
>                                        struct ext3_extent *from);

So one of the reasons why I've intentionally never exposed "struct
ext3_extent" in the libext2fs interface is because that's an on-disk
structure which I keep hoping we might change someday --- for example,
to allow for 64-bit logical block numbers so we can create ext4 files
greater than 2**32 blocks.  It might be that some other future
enhancement, such as say, reflinks (depending on how we implement
them), or reverse pointers, might also require making changes to the
on-disk format.

The kernel code has the on-disk format and the various logical
manipulations of the extent tree hopelessly entangled with each other,
which means changing the kernel code to support more than one on-disk
extent structure is going to be **hard**.  But in the userspace code,
all of the knowledge about the on-disk structure is abstracted away
inside lib/ext2fs/extent.c.

It may very well be that for fast commit, we're going to need to crack
open that abstraction barrier a bit.  But let's make sure the function
name makes it clear that what we are doing here is converting between
a particular on-disk encoding and the ext2fs abtract extent type.
"ext2fs_convert_extent" doesn't exactly make this clear.

It might also be that what should do is include a pointer to the fs
and inode structures, and call this something like
"ext2fs_{decode,encode}_extent()", and pass in the on-disk format via
a void *.  We might also want to have some kind of
ext2fs_validate_extent() function which takes a void * and validates
the on-disk structure to make sure it's sane.

What do you think?

					- Ted