linux-ext4 - Re: [RFC PATCH 1/3][e2fsprogs] Add 64-bit IO manager operations to struct_io

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] [day] [month] [year] [list]

Message-ID: <20080310144042.GE26651@mit.edu>
Date:	Mon, 10 Mar 2008 10:40:42 -0400
From:	Theodore Tso <tytso@....edu>
To:	"Jose R. Santos" <jrs@...ibm.com>
Cc:	linux-ext4@...r.kernel.org
Subject: Re: [RFC PATCH 1/3][e2fsprogs] Add 64-bit IO manager operations to
	struct_io_manager.

On Mon, Mar 03, 2008 at 10:41:18AM -0600, Jose R. Santos wrote:
> From: Jose R. Santos <jrs@...ibm.com>
> 
> Add 64-bit IO manager operations to struct_io_manager.
> 
> In order to provide 64-bit block support for IO managers an maintain
> ABI compatibility with the old API, some new functions need to be
> added to struct_io_manger.  Luckily, strcut_io_manager has some
> reserved space that we can use to add these new functions.

Looks good, and I won't NACK this patch just because of this.
However, be aware that:

> -	int		reserved[14];
> +	errcode_t (*read_blk64)(io_channel channel, unsigned long long block,
> +					int count, void *data);
> +	errcode_t (*write_blk64)(io_channel channel, unsigned long long block,
> +					int count, const void *data);
> +	int		reserved[12];
>  };

... this doesn't really work on x86_64 platforms since int's are 4
bytes and pointers are 8 bytes.

*Fortunately* it doesn't really matter here since the reserved block
is mainly to make sure that there are sufficient zero-filled bytes
after the structure so that there is little or no risk that if new
code which looks for a new function pointer like read_blk64 tries to
access that area after the structure, it will find zero's.  

As it turns out we don't ever expect the user to allocate io manager
structures, and in fact the primary place (and probably the only
place) where io_managers are defined is in libext2fs --- although if a
user were to define their own io_manager in their program, we would be
relying on the reserved fields to provide enough zero-filled slack
space to avoid problems.

In structures where it you really do need to maintain structure sizes,
it is very important to have a separate reserved_int[16] and
reserved_ptr[16] array --- or better yet, make the structure private
and not visible to the external callers, using accessor functions as
necessary, and use a callee (as opposed to caller) allocation
convention, which removes the need for the caller (i.e., calling
application) to know the size of the structure.

This is all not relevant to the patch at hand, though (although I will
probably not decrease the size of the reserved array to provide slack
for future expansions), but is the sort of thing that should go into a
wiki for e2fsprogs coding/style conventions one of these days.  :-)

     	 	   			    	   - Ted
--
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html