linux-kernel - Re: [PATCH V1 02/17] ext4: Add the basic function for inline data support.

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: <E38E92E9-2937-47BA-A9F4-B1F75C9FDE4A@dilger.ca>
Date:	Wed, 26 Oct 2011 02:36:56 -0600
From:	Andreas Dilger <adilger@...ger.ca>
To:	Tao Ma <tm@....ma>
Cc:	linux-ext4@...r.kernel.org, tytso@....edu,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH V1 02/17] ext4: Add the basic function for inline data support.

On 2011-10-26, at 1:34 AM, Tao Ma wrote:
> Implement inline data with xattr. This idea is inspired by Andreas.
> So now we use "system.data" to store xattr.  For inode_size = 256,
> currently we uses all the space between i_extra_isize and inode_size.
> For inode_size > 256, we use half of that space.
> 
> +#define EXT4_XATTR_SYSTEM_DATA_NAME		"data"

Did you check whether the "data" string takes 4 bytes or 8 in the
xattr header?  I believe it is storing the NUL termination (and
the code is using "sizeof(EXT4_XATTR_SYSTEM_DATA_NAME)" = 5), so
I believe it will round up to the next 4-byte boundary.  Using a
3-byte name will save a bit of space, like "system.dat".

> +void ext4_write_inline_data(struct inode *inode, struct ext4_iloc *iloc,
> +			    void *buffer, loff_t pos, unsigned len)
> +{
> +	header = IHDR(inode, ext4_raw_inode(iloc));
> +	entry = (struct ext4_xattr_entry *)((void *)ext4_raw_inode(iloc) +
> +					    EXT4_I(inode)->i_inline_off);
> +	memcpy((void *)IFIRST(header) + le16_to_cpu(entry->e_value_offs) + pos,
> +	       buffer + pos, len);
> +}
> +
> +int ext4_init_inline_data(handle_t *handle, struct inode *inode,
> +			  struct ext4_iloc *iloc)
> +{
> +	size = ext4_get_max_inline_size(inode);
> +	value = kzalloc(size, GFP_NOFS);
> +	if (!value)
> +		return -ENOMEM;
> +
> +	error = ext4_xattr_ibody_set(handle, inode, &i, &is);
> +}

Since file data is changed very rarely, instead of consuming the full
xattr space that may not be needed, wouldn't it be better to change
ext4_write_inline_data() to just to the ext4_xattr_ibody_set() to save
the exact-sized buffer into the xattr?  That will allow other xattrs
to be stored in this space as well as the inline data.


Cheers, Andreas





--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/