linux-ext4 - Re: [PATCH] debugfs: Add inline data feature for symlink.

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: <63e6941e-70ab-4fec-941c-f13f47585594@alibaba-inc.com>
Date:	Fri, 01 Aug 2014 18:37:56 +0800
From:	"侯普(谷熠)" <houpu.hp@...baba-inc.com>
To:	"Darrick J. Wong" <darrick.wong@...cle.com>
Cc:	"Ext4 Developers List" <linux-ext4@...r.kernel.org>
Subject: Re: [PATCH] debugfs: Add inline data feature for symlink.

On Thu, Jul 31, 2014 at 02:58:52PM +0800, Pu Hou wrote:
>> Symlink in debugfs can take advantage of inline data feature. The path name of the target of the symbol link which is longer than 60 byte and shorter than 132 byte can stay in inline data area.
>
>I think Ted prefers wrapping commit messages at 70 bytes.
>
Thanks for reminding me. I will fix it.

>> +	data.ea_size = size - EXT4_MIN_INLINE_DATA_SIZE;
>> +	data.ea_data = buf + EXT4_MIN_INLINE_DATA_SIZE;
>> +	return ext2fs_inline_data_ea_set(&data);
>
>Doesn't ext2fs_inline_data_set() suffice?
>
It is unnecessary to use this dedicated function.

>> +					sizeof(__u32)+
>> +					EXT4_MIN_INLINE_DATA_SIZE;
>> +			inline_link = (target_len < max_inline);
>
>Does ext2fs_xattr_inode_max_size() + EXT4_MIN_INLINE_DATA_SIZE return incorrect
>results?  I don't think we should have all xattr specific stuff belongs here in
>the symlink routines.

Sorry. I did not notice that some xattr might be existed  when an inode
number is passed to ext2fs_symlink(). But, as far as I can see, when 0
is pass as inode number. ext2fs_xattr_inode_max_size() will read inode
by ext2fs_read_inode_full(). But at that time, we have not writen the
inode back. and the size of inline data for symlink is uncertain.
I will try to find a workaround.

>> -		if (retval)
>> -			goto cleanup;
>> +		  retval = io_channel_write_blk64(fs->io, blk, 1, block_buf);
>> +		  if (retval)
>> +		    goto cleanup;
>
>Please fix the indentation inconsistency (tabs, not spaces).

>> -	if (!fastlink)
>> +	if ((!fastlink) && (!inline_link))
>
>Probably unnecessary to put !fastlink and !inline_link in parentheses.
>
>--D
Darrick, thank again for your generous help.
--
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