udf: fix possible leakage of blocks

We have to take care that when we call udf_discard_prealloc() from udf_clear_inode() we have to write inode ourselves afterwards (otherwise, some changes might be lost leading to leakage of blocks, use of free blocks or improperly aligned extents). Also udf_discard_prealloc() does two different things - it removes preallocated blocks and truncates the last extent to exactly match i_size. We move the latter functionality to udf_truncate_tail_extent(), call udf_discard_prealloc() when last reference to a file is dropped and call udf_truncate_tail_extent() when inode is being removed from inode cache (udf_clear_inode() call). We cannot call udf_truncate_tail_extent() earlier as subsequent open+write would find the last block of the file mapped and happily write to the end of it, although the last extent says it's shorter. [akpm@linux-foundation.org: Make checkpatch.pl happier] Signed-off-by: Jan Kara <jack@suse.cz> Cc: Eric Sandeen <sandeen@sandeen.net> Cc: Cyrill Gorcunov <gorcunov@gmail.com> Signed-off-by: Andrew Morton <akpm@linux-foundation.org> Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>

udf: fix possible leakage of blocks
We have to take care that when we call udf_discard_prealloc() from udf_clear_inode() we have to write inode ourselves afterwards (otherwise, some changes might be lost leading to leakage of blocks, use of free blocks or improperly aligned extents). Also udf_discard_prealloc() does two different things - it removes preallocated blocks and truncates the last extent to exactly match i_size. We move the latter functionality to udf_truncate_tail_extent(), call udf_discard_prealloc() when last reference to a file is dropped and call udf_truncate_tail_extent() when inode is being removed from inode cache (udf_clear_inode() call). We cannot call udf_truncate_tail_extent() earlier as subsequent open+write would find the last block of the file mapped and happily write to the end of it, although the last extent says it's shorter. [akpm@linux-foundation.org: Make checkpatch.pl happier] Signed-off-by: Jan Kara <jack@suse.cz> Cc: Eric Sandeen <sandeen@sandeen.net> Cc: Cyrill Gorcunov <gorcunov@gmail.com> Signed-off-by: Andrew Morton <akpm@linux-foundation.org> Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
Jan Kara · Linus Torvalds
1 parent 4b356be019
Showing 3 changed files with 74 additions and 17 deletions Side-by-side Diff
fs/udf/inode.c
fs/udf/truncate.c
fs/udf/udfdecl.h
@@ -100,14 +100,23 @@
 	clear_inode(inode);
 }
  
+/*
+ * If we are going to release inode from memory, we discard preallocation and
+ * truncate last inode extent to proper length. We could use drop_inode() but
+ * it's called under inode_lock and thus we cannot mark inode dirty there.  We
+ * use clear_inode() but we have to make sure to write inode as it's not written
+ * automatically.
+ */
 void udf_clear_inode(struct inode *inode)
 {
 	if (!(inode->i_sb->s_flags & MS_RDONLY)) {
 		lock_kernel();
+		/* Discard preallocation for directories, symlinks, etc. */
 		udf_discard_prealloc(inode);
+		udf_truncate_tail_extent(inode);
 		unlock_kernel();
+		write_inode_now(inode, 1);
 	}
-
 	kfree(UDF_I_DATA(inode));
 	UDF_I_DATA(inode) = NULL;
 }
@@ -61,7 +61,11 @@
 	}
 }
  
-void udf_discard_prealloc(struct inode * inode)
+/*
+ * Truncate the last extent to match i_size. This function assumes
+ * that preallocation extent is already truncated.
+ */
+void udf_truncate_tail_extent(struct inode *inode)
 {
 	struct extent_position epos = { NULL, 0, {0, 0}};
 	kernel_lb_addr eloc;
  
  
  
  
  
  
  
  
@@ -71,44 +75,87 @@
 	int adsize;
  
 	if (UDF_I_ALLOCTYPE(inode) == ICBTAG_FLAG_AD_IN_ICB ||
-		inode->i_size == UDF_I_LENEXTENTS(inode))
+	    inode->i_size == UDF_I_LENEXTENTS(inode))
 		return;
+	/* Are we going to delete the file anyway? */
+	if (inode->i_nlink == 0)
+		return;
  
 	if (UDF_I_ALLOCTYPE(inode) == ICBTAG_FLAG_AD_SHORT)
 		adsize = sizeof(short_ad);
 	else if (UDF_I_ALLOCTYPE(inode) == ICBTAG_FLAG_AD_LONG)
 		adsize = sizeof(long_ad);
 	else
-		adsize = 0;
+		BUG();
  
-	epos.block = UDF_I_LOCATION(inode);
-
 	/* Find the last extent in the file */
 	while ((netype = udf_next_aext(inode, &epos, &eloc, &elen, 1)) != -1)
 	{
 		etype = netype;
 		lbcount += elen;
-		if (lbcount > inode->i_size && lbcount - elen < inode->i_size)
-		{
-			WARN_ON(lbcount - inode->i_size >= inode->i_sb->s_blocksize);
+		if (lbcount > inode->i_size) {
+			if (lbcount - inode->i_size >= inode->i_sb->s_blocksize)
+				printk(KERN_WARNING
+				       "udf_truncate_tail_extent(): Too long "
+				       "extent after EOF in inode %u: i_size: "
+				       "%Ld lbcount: %Ld extent %u+%u\n",
+				       (unsigned)inode->i_ino,
+				       (long long)inode->i_size,
+				       (long long)lbcount,
+				       (unsigned)eloc.logicalBlockNum,
+				       (unsigned)elen);
 			nelen = elen - (lbcount - inode->i_size);
 			epos.offset -= adsize;
 			extent_trunc(inode, &epos, eloc, etype, elen, nelen);
 			epos.offset += adsize;
-			lbcount = inode->i_size;
+			if (udf_next_aext(inode, &epos, &eloc, &elen, 1) != -1)
+				printk(KERN_ERR "udf_truncate_tail_extent(): "
+				       "Extent after EOF in inode %u.\n",
+				       (unsigned)inode->i_ino);
+			break;
 		}
 	}
+	/* This inode entry is in-memory only and thus we don't have to mark
+	 * the inode dirty */
+	UDF_I_LENEXTENTS(inode) = inode->i_size;
+	brelse(epos.bh);
+}
+
+void udf_discard_prealloc(struct inode *inode)
+{
+	struct extent_position epos = { NULL, 0, {0, 0}};
+	kernel_lb_addr eloc;
+	uint32_t elen;
+	uint64_t lbcount = 0;
+	int8_t etype = -1, netype;
+	int adsize;
+
+	if (UDF_I_ALLOCTYPE(inode) == ICBTAG_FLAG_AD_IN_ICB ||
+		inode->i_size == UDF_I_LENEXTENTS(inode))
+		return;
+
+	if (UDF_I_ALLOCTYPE(inode) == ICBTAG_FLAG_AD_SHORT)
+		adsize = sizeof(short_ad);
+	else if (UDF_I_ALLOCTYPE(inode) == ICBTAG_FLAG_AD_LONG)
+		adsize = sizeof(long_ad);
+	else
+		adsize = 0;
+
+	epos.block = UDF_I_LOCATION(inode);
+
+	/* Find the last extent in the file */
+	while ((netype = udf_next_aext(inode, &epos, &eloc, &elen, 1)) != -1) {
+		etype = netype;
+		lbcount += elen;
+	}
 	if (etype == (EXT_NOT_RECORDED_ALLOCATED >> 30)) {
 		epos.offset -= adsize;
 		lbcount -= elen;
 		extent_trunc(inode, &epos, eloc, etype, elen, 0);
-		if (!epos.bh)
-		{
+		if (!epos.bh) {
 			UDF_I_LENALLOC(inode) = epos.offset - udf_file_entry_alloc_offset(inode);
 			mark_inode_dirty(inode);
-		}
-		else
-		{
+		} else {
 			struct allocExtDesc *aed = (struct allocExtDesc *)(epos.bh->b_data);
 			aed->lengthAllocDescs = cpu_to_le32(epos.offset - sizeof(struct allocExtDesc));
 			if (!UDF_QUERY_FLAG(inode->i_sb, UDF_FLAG_STRICT) || UDF_SB_UDFREV(inode->i_sb) >= 0x0201)
  
@@ -118,9 +165,9 @@
 			mark_buffer_dirty_inode(epos.bh, inode);
 		}
 	}
+	/* This inode entry is in-memory only and thus we don't have to mark
+	 * the inode dirty */
 	UDF_I_LENEXTENTS(inode) = lbcount;
-
-	WARN_ON(lbcount != inode->i_size);
 	brelse(epos.bh);
 }
  
@@ -146,6 +146,7 @@
 extern struct inode * udf_new_inode (struct inode *, int, int *);
  
 /* truncate.c */
+extern void udf_truncate_tail_extent(struct inode *);
 extern void udf_discard_prealloc(struct inode *);
 extern void udf_truncate_extents(struct inode *);
...	...	@@ -100,14 +100,23 @@
100	100	clear_inode(inode);
101	101	}
102	102
	103	+/*
	104	+ * If we are going to release inode from memory, we discard preallocation and
	105	+ * truncate last inode extent to proper length. We could use drop_inode() but
	106	+ * it's called under inode_lock and thus we cannot mark inode dirty there. We
	107	+ * use clear_inode() but we have to make sure to write inode as it's not written
	108	+ * automatically.
	109	+ */
103	110	void udf_clear_inode(struct inode *inode)
104	111	{
105	112	if (!(inode->i_sb->s_flags & MS_RDONLY)) {
106	113	lock_kernel();
	114	+ /* Discard preallocation for directories, symlinks, etc. */
107	115	udf_discard_prealloc(inode);
	116	+ udf_truncate_tail_extent(inode);
108	117	unlock_kernel();
	118	+ write_inode_now(inode, 1);
109	119	}
110		-
111	120	kfree(UDF_I_DATA(inode));
112	121	UDF_I_DATA(inode) = NULL;
113	122	}
...	...	@@ -61,7 +61,11 @@
61	61	}
62	62	}
63	63
64		-void udf_discard_prealloc(struct inode * inode)
	64	+/*
	65	+ * Truncate the last extent to match i_size. This function assumes
	66	+ * that preallocation extent is already truncated.
	67	+ */
	68	+void udf_truncate_tail_extent(struct inode *inode)
65	69	{
66	70	struct extent_position epos = { NULL, 0, {0, 0}};
67	71	kernel_lb_addr eloc;
68	72
69	73
70	74
71	75
72	76
73	77
74	78
75	79
...	...	@@ -71,44 +75,87 @@
71	75	int adsize;
72	76
73	77	if (UDF_I_ALLOCTYPE(inode) == ICBTAG_FLAG_AD_IN_ICB \|\|
74		- inode->i_size == UDF_I_LENEXTENTS(inode))
	78	+ inode->i_size == UDF_I_LENEXTENTS(inode))
75	79	return;
	80	+ /* Are we going to delete the file anyway? */
	81	+ if (inode->i_nlink == 0)
	82	+ return;
76	83
77	84	if (UDF_I_ALLOCTYPE(inode) == ICBTAG_FLAG_AD_SHORT)
78	85	adsize = sizeof(short_ad);
79	86	else if (UDF_I_ALLOCTYPE(inode) == ICBTAG_FLAG_AD_LONG)
80	87	adsize = sizeof(long_ad);
81	88	else
82		- adsize = 0;
	89	+ BUG();
83	90
84		- epos.block = UDF_I_LOCATION(inode);
85		-
86	91	/* Find the last extent in the file */
87	92	while ((netype = udf_next_aext(inode, &epos, &eloc, &elen, 1)) != -1)
88	93	{
89	94	etype = netype;
90	95	lbcount += elen;
91		- if (lbcount > inode->i_size && lbcount - elen < inode->i_size)
92		- {
93		- WARN_ON(lbcount - inode->i_size >= inode->i_sb->s_blocksize);
	96	+ if (lbcount > inode->i_size) {
	97	+ if (lbcount - inode->i_size >= inode->i_sb->s_blocksize)
	98	+ printk(KERN_WARNING
	99	+ "udf_truncate_tail_extent(): Too long "
	100	+ "extent after EOF in inode %u: i_size: "
	101	+ "%Ld lbcount: %Ld extent %u+%u\n",
	102	+ (unsigned)inode->i_ino,
	103	+ (long long)inode->i_size,
	104	+ (long long)lbcount,
	105	+ (unsigned)eloc.logicalBlockNum,
	106	+ (unsigned)elen);
94	107	nelen = elen - (lbcount - inode->i_size);
95	108	epos.offset -= adsize;
96	109	extent_trunc(inode, &epos, eloc, etype, elen, nelen);
97	110	epos.offset += adsize;
98		- lbcount = inode->i_size;
	111	+ if (udf_next_aext(inode, &epos, &eloc, &elen, 1) != -1)
	112	+ printk(KERN_ERR "udf_truncate_tail_extent(): "
	113	+ "Extent after EOF in inode %u.\n",
	114	+ (unsigned)inode->i_ino);
	115	+ break;
99	116	}
100	117	}
	118	+ /* This inode entry is in-memory only and thus we don't have to mark
	119	+ * the inode dirty */
	120	+ UDF_I_LENEXTENTS(inode) = inode->i_size;
	121	+ brelse(epos.bh);
	122	+}
	123	+
	124	+void udf_discard_prealloc(struct inode *inode)
	125	+{
	126	+ struct extent_position epos = { NULL, 0, {0, 0}};
	127	+ kernel_lb_addr eloc;
	128	+ uint32_t elen;
	129	+ uint64_t lbcount = 0;
	130	+ int8_t etype = -1, netype;
	131	+ int adsize;
	132	+
	133	+ if (UDF_I_ALLOCTYPE(inode) == ICBTAG_FLAG_AD_IN_ICB \|\|
	134	+ inode->i_size == UDF_I_LENEXTENTS(inode))
	135	+ return;
	136	+
	137	+ if (UDF_I_ALLOCTYPE(inode) == ICBTAG_FLAG_AD_SHORT)
	138	+ adsize = sizeof(short_ad);
	139	+ else if (UDF_I_ALLOCTYPE(inode) == ICBTAG_FLAG_AD_LONG)
	140	+ adsize = sizeof(long_ad);
	141	+ else
	142	+ adsize = 0;
	143	+
	144	+ epos.block = UDF_I_LOCATION(inode);
	145	+
	146	+ /* Find the last extent in the file */
	147	+ while ((netype = udf_next_aext(inode, &epos, &eloc, &elen, 1)) != -1) {
	148	+ etype = netype;
	149	+ lbcount += elen;
	150	+ }
101	151	if (etype == (EXT_NOT_RECORDED_ALLOCATED >> 30)) {
102	152	epos.offset -= adsize;
103	153	lbcount -= elen;
104	154	extent_trunc(inode, &epos, eloc, etype, elen, 0);
105		- if (!epos.bh)
106		- {
	155	+ if (!epos.bh) {
107	156	UDF_I_LENALLOC(inode) = epos.offset - udf_file_entry_alloc_offset(inode);
108	157	mark_inode_dirty(inode);
109		- }
110		- else
111		- {
	158	+ } else {
112	159	struct allocExtDesc aed = (struct allocExtDesc )(epos.bh->b_data);
113	160	aed->lengthAllocDescs = cpu_to_le32(epos.offset - sizeof(struct allocExtDesc));
114	161	if (!UDF_QUERY_FLAG(inode->i_sb, UDF_FLAG_STRICT) \|\| UDF_SB_UDFREV(inode->i_sb) >= 0x0201)
115	162
...	...	@@ -118,9 +165,9 @@
118	165	mark_buffer_dirty_inode(epos.bh, inode);
119	166	}
120	167	}
	168	+ /* This inode entry is in-memory only and thus we don't have to mark
	169	+ * the inode dirty */
121	170	UDF_I_LENEXTENTS(inode) = lbcount;
122		-
123		- WARN_ON(lbcount != inode->i_size);
124	171	brelse(epos.bh);
125	172	}
126	173
...	...	@@ -146,6 +146,7 @@
146	146	extern struct inode * udf_new_inode (struct inode , int, int );
147	147
148	148	/* truncate.c */
	149	+extern void udf_truncate_tail_extent(struct inode *);
149	150	extern void udf_discard_prealloc(struct inode *);
150	151	extern void udf_truncate_extents(struct inode *);
151	152