Skip to contentSkip to conversation

reiserfs: Fix corruption during shrinking of xattrs

PATCH In review Jeff Mahoney 2 participants 3 messages Apr 13, 2010 – Apr 15, 2010

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 reiserfs: Fix corruption during shrinking of xattrs

Commit 48b32a3553a54740d236b79a90f20147a25875e3 introduced a problem that causes corruption when extended attributes are replaced with a smaller value.

The issue is that the reiserfs_setattr to shrink the xattr file was moved from before the write to after the write.

The root issue has always been in the reiserfs xattr code, but was papered over by the fact that in the shrink case, the file would just be expanded again while the xattr was written.

The end result is that the last 8 bytes of xattr data are lost.

This patch fixes it to use new_size.

This issue was reported at: https://bugzilla.kernel.org/show_bug.cgi?id=14826

---

 fs/reiserfs/xattr.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
fs/reiserfs/xattr.c +1 -1
@@ -553,7 +560,7 @@ reiserfs_xattr_set_handle(struct reiserf
 	if (!err && new_size < i_size_read(dentry->d_inode)) {
 		struct iattr newattrs = {
 			.ia_ctime = current_fs_time(inode->i_sb),
-			.ia_size = buffer_size,
+			.ia_size = new_size,
 			.ia_valid = ATTR_SIZE | ATTR_CTIME,
 		};
 
Signature · 2 lines
Jeff Mahoney
SUSE Labs
Tested-by on v1 1/1
#1 lore raw
Quoted 1 line · Jeff Mahoney

This patch fixes it to use new_size.

With this patch applied (to today's mainline git, 2.6.34-rc4) I was able to create 30k files with xattrs on the same test system that was failing to do so before.

Thanks for the fix!

Christian.

Signature · 3 lines
BOFH excuse #88:

Boss' kid fucked up the machine
fs/reiserfs/xattr.c +1 -1
index 4f9586b..2058c28 100644
@@ -554,7 +554,7 @@ reiserfs_xattr_set_handle(struct reiserfs_transaction_handle *th,
 	if (!err && new_size < i_size_read(dentry->d_inode)) {
 		struct iattr newattrs = {
 			.ia_ctime = current_fs_time(inode->i_sb),
-			.ia_size = buffer_size,
+			.ia_size = new_size,
 			.ia_valid = ATTR_SIZE | ATTR_CTIME,
 		};
 
#2 lore raw
Quoted 4 lines · Christian Kujau
Quoted 1 line · Jeff Mahoney

This patch fixes it to use new_size.

Dang. While this *does* seem to fix the "Input/output error after a large amount of files have their xattrs removed", it does not fix the initial issue in #14826, where occasionally this jdm-20002 message shows up.

I just tested this: I still have this calendarserver (caldavd) and it's heavily using xattr. I have removed everything under /var/spool/caldavd, started caldavd and am now importing a calendar. caldavd will now create files under /var/spool/caldavd, wil assign xattrs and read them. And just after a few operations, this happens again:

[138794.184926] REISERFS warning (device xvda3): jdm-20002 

reiserfs_xattr_get: Invalid hash for xattr (user.{http:%2F%2Ftwistedmatrix.com%2Fxml_namespace%2Fdav%2Fprivate%2F}quota-used) associated with [765 3282 0x0 SD]

[139238.740552] REISERFS warning (device xvda3): jdm-20002 

reiserfs_xattr_get: Invalid hash for xattr (user.{DAV:}displayname) associated with [765 3402 0x0 SD]

Inodes 3282, 3402, aka

./calendars/__uids__/afebe2f1-f51a-5859-af15-f22312ad6634 ./calendars/__uids__/afebe2f1-f51a-5859-af15-f22312ad6634/2CC21268-C87E-4AE6-AFD4-33946D5621AB

....under /var/spool/caldavd. So it does happen for new files (directories in this case) indeed.

However, there's no file corruption, no I/O errors - I can still access the directory and its contents. The xattr information might be lost though. Bug #14826 somehow transformed into "my filesystem is corrupt", as the bug with the large amount of files/xattr seems to generate the same message in the syslog. But that's a different issue, methinks.

Christian.

[0] https://bugzilla.kernel.org/show_bug.cgi?id=14826

Signature · 4 lines
BOFH excuse #124:


user to computer ration too low.