Skip to contentSkip to conversation

reiserfs fix for null bytes in small files

PATCH In review Chris Mason 3 participants 4 messages Feb 16, 2001

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 reiserfs fix for null bytes in small files

Hello everyone,

I think Alexander Zarochentcev and I have finally figured out cause for null bytes in small reiserfs files. reiserfs stores parts of these files packed together in the tree, and the packed bytes can shift around as the tree is balanced.

When converting from the packed bytes to a full block, the full block is inserted, and the packed bytes are removed. If the packed bytes are split between two tree blocks, and then merged by a different process balancing the tree, the conversion code might remove too many bytes from the file. This creates a hole in the file, which is why people see null bytes.

If anyone wants more details, drop a line to me or the reiserfs list ;-) This patch against 2.4.1 (will work on any 2.4.1ac or 2.4.2pre as well) should fix it, please try it on your non-production machines, we are still running it through tests here.

-chris

fs/reiserfs/tail_conversion.c renamed from fs/reiserfs/tail_conversion.c.old +6 -8
@@ -92,7 +92,7 @@
     /* Move bytes from the direct items to the new unformatted node
        and delete them. */
     while (1)  {
-	int item_len, first_direct;
+	int tail_size;
 
 	/* end_key.k_offset is set so, that we will always have found
            last item of the file */
@@ -103,13 +103,11 @@
 #ifdef CONFIG_REISERFS_CHECK
 	if (!is_direct_le_ih (p_le_ih))
 	    reiserfs_panic (sb, "vs-14055: direct2indirect: "
-			    "direct item expected, found %h", p_le_ih);
+			    "direct item expected(%k), found %h", 
+				&end_key, p_le_ih);
 #endif
-	if ((le_ih_k_offset (p_le_ih) & (n_blk_size - 1)) == 1)
-	    first_direct = 1;
-	else
-	    first_direct = 0;
-	item_len = le16_to_cpu (p_le_ih->ih_item_len);
+	tail_size = (le_ih_k_offset (p_le_ih) & (n_blk_size - 1))
+	    + ih_item_len(p_le_ih) - 1;
 
 	/* we only send the unbh pointer if the buffer is not up to date.
 	** this avoids overwriting good data from writepage() with old data
@@ -123,7 +121,7 @@
 	n_retval = reiserfs_delete_item (th, path, &end_key, inode, 
 	                                 up_to_date_bh) ;
 
-	if (first_direct && item_len == n_retval)
+	if (tail_size == n_retval)
 	    // done: file does not have direct items anymore
 	    break;
 
#1 lore raw
Quoted 4 lines · Chris Mason

Hello everyone,

I think Alexander Zarochentcev and I have finally figured out cause for null bytes in small reiserfs files.

Alexander&Chris, you are the masters! :-) (Yet the others from the reiserfs team, you are the masters too ;-))

Can you post a message when a kernel (ac|pre)release is appearing with that patch applied? I'm more than willing to test.

Xuân.

#2 lore raw
Quoted 6 lines · On Friday, February 16, 2001 05:01:39 PM +0100 Xuan Baldauf…
Quoted 2 lines · Chris Mason

Hello everyone,

Quoted 9 lines
Quoted 2 lines

I think Alexander Zarochentcev and I have finally figured out cause for null bytes in small reiserfs files.

Alexander&Chris, you are the masters! :-) (Yet the others from the reiserfs team, you are the masters too ;-))

Can you post a message when a kernel (ac|pre)release is appearing with that patch applied? I'm more than willing to test.

Sure. Alan already offered to toss it into an ac release, he is waiting on confirmation from me that we are happy with the patch. I'm not expecting any problems, but I'd like to see it tested on a few other machines before it gets included (it ran overnight under load on my boxes).

-chris

#3 lore raw
In reply to Chris Mason

Hi

Quoted 14 lines · Chris Mason

Hello everyone,

I think Alexander Zarochentcev and I have finally figured out cause for null bytes in small reiserfs files. reiserfs stores parts of these files packed together in the tree, and the packed bytes can shift around as the tree is balanced.

When converting from the packed bytes to a full block, the full block is inserted, and the packed bytes are removed. If the packed bytes are split between two tree blocks, and then merged by a different process balancing the tree, the conversion code might remove too many bytes from the file. This creates a hole in the file, which is why people see null bytes.

Quoted 6 lines

If anyone wants more details, drop a line to me or the reiserfs list ;-) This patch against 2.4.1 (will work on any 2.4.1ac or 2.4.2pre as well) should fix it, please try it on your non-production machines, we are still running it through tests here.

Just to make things clear:
reiserfs for 2.2 does not have this bug, so there is nothing to fix.

Thanks,
vs

Quoted 41 lines
(quoted diff) +0 -1
-chris
fs/reiserfs/tail_conversion.c Thu Feb 15 13:10:23 2001 renamed from fs/reiserfs/tail_conversion.c.old Thu Feb 15 13:16:47 2001 +6 -8
@@ -92,7 +92,7 @@
     /* Move bytes from the direct items to the new unformatted node
        and delete them. */
     while (1)  {
-       int item_len, first_direct;
+       int tail_size;
 
        /* end_key.k_offset is set so, that we will always have found
            last item of the file */
@@ -103,13 +103,11 @@
 #ifdef CONFIG_REISERFS_CHECK
        if (!is_direct_le_ih (p_le_ih))
            reiserfs_panic (sb, "vs-14055: direct2indirect: "
-                           "direct item expected, found %h", p_le_ih);
+                           "direct item expected(%k), found %h",
+                               &end_key, p_le_ih);
 #endif
-       if ((le_ih_k_offset (p_le_ih) & (n_blk_size - 1)) == 1)
-           first_direct = 1;
-       else
-           first_direct = 0;
-       item_len = le16_to_cpu (p_le_ih->ih_item_len);
+       tail_size = (le_ih_k_offset (p_le_ih) & (n_blk_size - 1))
+           + ih_item_len(p_le_ih) - 1;
 
        /* we only send the unbh pointer if the buffer is not up to date.
        ** this avoids overwriting good data from writepage() with old data
@@ -123,7 +121,7 @@
        n_retval = reiserfs_delete_item (th, path, &end_key, inode,
                                         up_to_date_bh) ;
 
-       if (first_direct && item_len == n_retval)
+       if (tail_size == n_retval)
            // done: file does not have direct items anymore
            break;