Skip to contentSkip to conversation

[2.6] Make reiserfs not to crash on oom

PATCH In review Oleg Drokin 3 participants 5 messages May 12, 2004

lore
Share & tools
lore

Oleg Drokin

author
#0 lore raw

PATCH 1/1 [2.6] Make reiserfs not to crash on oom

Hello!

Thanks to Standford guys, a case where reiserfs can dereference NULL pointer if memory allocation fail during mount was identified.

Here's 2.6 version of patch.

Bye,
Oleg

===== fs/reiserfs/journal.c 1.91 vs edited =====

fs/reiserfs/journal.c +4 -2
@@ -2260,8 +2260,10 @@
     INIT_LIST_HEAD (&SB_JOURNAL(p_s_sb)->j_prealloc_list);
     INIT_LIST_HEAD(&SB_JOURNAL(p_s_sb)->j_working_list);
     INIT_LIST_HEAD(&SB_JOURNAL(p_s_sb)->j_journal_list);
-    reiserfs_allocate_list_bitmaps(p_s_sb, SB_JOURNAL(p_s_sb)->j_list_bitmap, 
- 				   SB_BMAP_NR(p_s_sb)) ;
+    if (reiserfs_allocate_list_bitmaps(p_s_sb,
+				       SB_JOURNAL(p_s_sb)->j_list_bitmap, 
+ 				       SB_BMAP_NR(p_s_sb)))
+	goto free_and_return ;
     allocate_bitmap_nodes(p_s_sb) ;
 
     /* reserved for journal area support */
#1 lore raw
Quoted 6 lines · Oleg Drokin

Hello!

Thanks to Standford guys, a case where reiserfs can dereference NULL pointer if memory allocation fail during mount was identified.

fs/reiserfs/journal.c line 2267

Quoted 10 lines
fs/reiserfs/journal.c +4 -2
@@ -2260,8 +2260,10 @@
     INIT_LIST_HEAD (&SB_JOURNAL(p_s_sb)->j_prealloc_list);
     INIT_LIST_HEAD(&SB_JOURNAL(p_s_sb)->j_working_list);
     INIT_LIST_HEAD(&SB_JOURNAL(p_s_sb)->j_journal_list);
-    reiserfs_allocate_list_bitmaps(p_s_sb, SB_JOURNAL(p_s_sb)->j_list_bitmap, 
- 				   SB_BMAP_NR(p_s_sb)) ;
+    if (reiserfs_allocate_list_bitmaps(p_s_sb,
+				       SB_JOURNAL(p_s_sb)->j_list_bitmap, 
+ 				       SB_BMAP_NR(p_s_sb)))
+	goto free_and_return ;
     allocate_bitmap_nodes(p_s_sb) ;

Are you leaking the 'journal' allocation here?
(Ditto some of the other failure paths too)

There's also a typod 'jornal' a few lines further down.

Dave

author
#2 lore raw

Hello!

Quoted 16 lines · Dave Jones
Quoted 4 lines

Thanks to Standford guys, a case where reiserfs can dereference NULL pointer if memory allocation fail during mount was identified.

fs/reiserfs/journal.c line 2267

Quoted 10 lines
fs/reiserfs/journal.c +4 -2
@@ -2260,8 +2260,10 @@
     INIT_LIST_HEAD (&SB_JOURNAL(p_s_sb)->j_prealloc_list);
     INIT_LIST_HEAD(&SB_JOURNAL(p_s_sb)->j_working_list);
     INIT_LIST_HEAD(&SB_JOURNAL(p_s_sb)->j_journal_list);
-    reiserfs_allocate_list_bitmaps(p_s_sb, SB_JOURNAL(p_s_sb)->j_list_bitmap, 
- 				   SB_BMAP_NR(p_s_sb)) ;
+    if (reiserfs_allocate_list_bitmaps(p_s_sb,
+				       SB_JOURNAL(p_s_sb)->j_list_bitmap, 
+ 				       SB_BMAP_NR(p_s_sb)))
+	goto free_and_return ;
     allocate_bitmap_nodes(p_s_sb) ;

Are you leaking the 'journal' allocation here?
(Ditto some of the other failure paths too)

No, there is "vfree(SB_JOURNAL(p_s_sb)) ;" at the end of free_journal_ram() that is called if we jump to that free_and_return label.

Quoted 1 line

There's also a typod 'jornal' a few lines further down.

Yup. Fortunatelly it does not break anything ;)
If somebody to catch all typos and misspellings in reiserfs code, that would worth a separate patch (and it will be big).

Bye,
Oleg

#3 lore raw
Quoted 3 lines · Oleg Drokin
Quoted 1 line

(Ditto some of the other failure paths too)

No, there is "vfree(SB_JOURNAL(p_s_sb)) ;" at the end of free_journal_ram() that is called if we jump to that free_and_return label.

Ah, I overlooked the double assignment of the vmalloc result 8)

Dave

#4 lore raw
In reply to Oleg Drokin
Quoted 6 lines · Oleg Drokin

Hello!

Thanks to Standford guys, a case where reiserfs can dereference NULL pointer if memory allocation fail during mount was identified.

Here's 2.6 version of patch.

Thanks Oleg, this looks right.

-chris