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 <steve@xxxxxxxxxxxxxxx> > > --- 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 <jack@xxxxxxxx> SUSE Labs, CR
