On Wed 14-08-19 07:50:02, Steven J. Magnani wrote:
> Windows is capable of creating UDF files having named streams.
> One example is the "Zone.Identifier" stream attached automatically
> to files downloaded from a network. See:
> https://msdn.microsoft.com/en-us/library/dn392609.aspx
>
> Modification of a file having one or more named streams in Linux causes
> the stream directory to become detached from the file, essentially leaking
> all blocks pertaining to the file's streams.
>
> Fix by saving off information about an inode's streams when reading it,
> for later use when its on-disk data is updated.
Thanks for the patch! I agree with the idea of this patch. Just some
small comments below.
> Changes from v1:
> Remove modifications that would limit leakage of all inode blocks
> on deletion.
> This restricts the patch to preservation of stream data during inode
> modification.
Please put patch changelog below Signed-off-by and --- delimiter so that it
does not get included in the final commit message when I do git-am.
> Signed-off-by: Steven J. Magnani <st...@digidescorp.com>
>
> --- a/fs/udf/udf_i.h 2019-07-26 11:35:28.257563879 -0500
> +++ b/fs/udf/udf_i.h 2019-08-06 14:35:55.579654263 -0500
> @@ -42,12 +42,15 @@ struct udf_inode_info {
> unsigned i_efe : 1; /* extendedFileEntry */
> unsigned i_use : 1; /* unallocSpaceEntry */
> unsigned i_strat4096 : 1;
> - unsigned reserved : 26;
> + unsigned i_streamdir : 1;
> + unsigned reserved : 25;
> union {
> struct short_ad *i_sad;
> struct long_ad *i_lad;
> __u8 *i_data;
> } i_ext;
> + struct kernel_lb_addr i_locStreamdir;
> + __u64 i_lenStreams;
> struct rw_semaphore i_data_sem;
> struct udf_ext_cache cached_extent;
> /* Spinlock for protecting extent cache */
> --- a/fs/udf/super.c 2019-07-26 11:35:28.253563792 -0500
> +++ b/fs/udf/super.c 2019-08-06 15:04:30.851086957 -0500
> @@ -151,9 +151,13 @@ static struct inode *udf_alloc_inode(str
>
> ei->i_unique = 0;
> ei->i_lenExtents = 0;
> + ei->i_lenStreams = 0;
> ei->i_next_alloc_block = 0;
> ei->i_next_alloc_goal = 0;
> ei->i_strat4096 = 0;
> + ei->i_streamdir = 0;
> + ei->i_locStreamdir.logicalBlockNum = 0xFFFFFFFF;
> + ei->i_locStreamdir.partitionReferenceNum = 0xFFFF;
I don't think you need to initialize i_locStreamdir when i_streamdir is
already set to 0...
> init_rwsem(&ei->i_data_sem);
> ei->cached_extent.lstart = -1;
> spin_lock_init(&ei->i_extent_cache_lock);
> --- a/fs/udf/inode.c 2019-07-26 11:35:28.253563792 -0500
> +++ b/fs/udf/inode.c 2019-08-06 15:04:30.851086957 -0500
> @@ -1485,6 +1485,10 @@ reread:
> iinfo->i_lenEAttr = le32_to_cpu(fe->lengthExtendedAttr);
> iinfo->i_lenAlloc = le32_to_cpu(fe->lengthAllocDescs);
> iinfo->i_checkpoint = le32_to_cpu(fe->checkpoint);
> + iinfo->i_streamdir = 0;
> + iinfo->i_lenStreams = 0;
> + iinfo->i_locStreamdir.logicalBlockNum = 0xFFFFFFFF;
> + iinfo->i_locStreamdir.partitionReferenceNum = 0xFFFF;
Ditto here...
> } else {
> inode->i_blocks = le64_to_cpu(efe->logicalBlocksRecorded) <<
> (inode->i_sb->s_blocksize_bits - 9);
> @@ -1498,6 +1502,16 @@ reread:
> iinfo->i_lenEAttr = le32_to_cpu(efe->lengthExtendedAttr);
> iinfo->i_lenAlloc = le32_to_cpu(efe->lengthAllocDescs);
> iinfo->i_checkpoint = le32_to_cpu(efe->checkpoint);
> +
> + /* Named streams */
> + iinfo->i_streamdir = (efe->streamDirectoryICB.extLength != 0);
> + iinfo->i_locStreamdir =
> + lelb_to_cpu(efe->streamDirectoryICB.extLocation);
> + iinfo->i_lenStreams = le64_to_cpu(efe->objectSize);
> + if (iinfo->i_lenStreams >= inode->i_size)
> + iinfo->i_lenStreams -= inode->i_size;
> + else
> + iinfo->i_lenStreams = 0;
Hum, maybe you could just have i_objectSize instead of i_lenStreams? You
use the field just to preserve objectSize anyway so there's no point in
complicating it.
Honza
--
Jan Kara <j...@suse.com>
SUSE Labs, CR
