187221 – [su+j] [patch] fsck_ufs -p segmentation fault with SU+J

Bug 187221 - [su+j] [patch] fsck_ufs -p segmentation fault with SU+J

Summary: [su+j] [patch] fsck_ufs -p segmentation fault with SU+J

Status:	Closed FIXED

Alias:	None

Product:	Base System
Classification:	Unclassified
Component:	bin (show other bugs)
Version:	9.2-STABLE
Hardware:	Any Any

Importance:	Normal Affects Only Me
Assignee:	Kirk McKusick

URL:
Keywords:

Depends on:
Blocks:

Reported:	2014-03-03 12:00 UTC by lampa
Modified:	2014-03-22 14:38 UTC (History)
CC List:	0 users

See Also:

Attachments
Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.

Description lampa 2014-03-03 12:00:00 UTC

Some mismatch in SU+J journal was found during initial fsck -p 
and it ends with segmentaion fault. Full fsck -y run without problems.

(gdb) where
#0  0x0000000000404fb0 in ckfini (markclean=0)
    at /usr/src/sbin/fsck_ffs/fsutil.c:418
#1  0x000000000040548b in pfatal (fmt=<value optimized out>)
    at /usr/src/sbin/fsck_ffs/fsutil.c:980
#2  0x00000000004047e1 in reply (question=0x417d07 "FALLBACK TO FULL FSCK")
    at /usr/src/sbin/fsck_ffs/fsutil.c:106
#3  0x0000000000413462 in suj_check (filesys=0x41007928 "/dev/mirror/root")
    at /usr/src/sbin/fsck_ffs/suj.c:2685
#4  0x0000000000408ca8 in main (argc=8193, argv=0x7fffffffdc50)
    at /usr/src/sbin/fsck_ffs/main.c:403
(gdb) frame 0
#0  0x0000000000404fb0 in ckfini (markclean=0)
    at /usr/src/sbin/fsck_ffs/fsutil.c:418
418                     if (cgbufs[cnt].b_un.b_cg == NULL)
(gdb) l
413                     free((char *)bp);
414             }
415             if (numbufs != cnt)
416                     errx(EEXIT, "panic: lost %d buffers", numbufs - cnt);
417             for (cnt = 0; cnt < sblock.fs_ncg; cnt++) {
418                     if (cgbufs[cnt].b_un.b_cg == NULL)
419                             continue;
420                     flush(fswritefd, &cgbufs[cnt]);
421                     free(cgbufs[cnt].b_un.b_cg);
422             }
 p cgbufs
$4 = (struct bufarea *) 0x0
(gdb)

It seems that cgbufs were not allocated, but they are flushed in ckfini() in
this case. This look similar problem like 
http://docs.freebsd.org/cgi/getmsg.cgi?fetch=478954+0+current/svn-src-head

Comment 1 lampa 2014-03-10 11:43:11 UTC

Simple fix, fsck -p runs without sigsegv now:

--- fsutil.c    2013-07-10 17:04:10.000000000 +0200
+++ fsutil.c.my 2014-03-10 12:33:55.000000000 +0100
@@ -414,13 +414,15 @@
        }
        if (numbufs != cnt)
                errx(EEXIT, "panic: lost %d buffers", numbufs - cnt);
-       for (cnt = 0; cnt < sblock.fs_ncg; cnt++) {
-               if (cgbufs[cnt].b_un.b_cg == NULL)
-                       continue;
-               flush(fswritefd, &cgbufs[cnt]);
-               free(cgbufs[cnt].b_un.b_cg);
+       if (cgbufs) {
+               for (cnt = 0; cnt < sblock.fs_ncg; cnt++) {
+                       if (cgbufs[cnt].b_un.b_cg == NULL)
+                               continue;
+                       flush(fswritefd, &cgbufs[cnt]);
+                       free(cgbufs[cnt].b_un.b_cg);
+               }
+               free(cgbufs);
        }
-       free(cgbufs);
        pbp = pdirbp = (struct bufarea *)0;
        if (cursnapshot == 0 && sblock.fs_clean != markclean) {
                if ((sblock.fs_clean = markclean) != 0) {

Petr Lampa

-- 
Computer Centre                             E-mail: lampa@fit.vutbr.cz
Faculty of Information Technology           Web: http://www.fit.vutbr.cz/
Brno University of Technology               Fax:  +420 54114-1270
Bozetechova 2, 612 66 Brno, Czech Republic  Phone: +420 54114-1225

Comment 2 Mark Linimon freebsd_committer

2014-03-10 13:56:34 UTC

Responsible Changed
From-To: freebsd-bugs->freebsd-fs

since this has a patch, assign it to fs@.

Comment 3 Kirk McKusick freebsd_committer

2014-03-12 01:12:43 UTC

Responsible Changed
From-To: freebsd-fs->mckusick

I will take this one.

Comment 4 dfilter service freebsd_committer

2014-03-12 01:28:29 UTC

Author: mckusick
Date: Wed Mar 12 01:28:21 2014
New Revision: 263062
URL: http://svnweb.freebsd.org/changeset/base/263062

Log:
  Avoid segment fault when attempting to clean up cylinder group
  buffer cache.
  
  PR:		187221
  Submitted by:	Petr Lampa <lampa@fit.vutbr.cz>
  Obtained from:	Petr Lampa <lampa@fit.vutbr.cz>
  MFC after:	1 week

Modified:
  head/sbin/fsck_ffs/fsutil.c

Modified: head/sbin/fsck_ffs/fsutil.c
==============================================================================
--- head/sbin/fsck_ffs/fsutil.c	Tue Mar 11 23:54:13 2014	(r263061)
+++ head/sbin/fsck_ffs/fsutil.c	Wed Mar 12 01:28:21 2014	(r263062)
@@ -436,13 +436,15 @@ ckfini(int markclean)
 	}
 	if (numbufs != cnt)
 		errx(EEXIT, "panic: lost %d buffers", numbufs - cnt);
-	for (cnt = 0; cnt < sblock.fs_ncg; cnt++) {
-		if (cgbufs[cnt].b_un.b_cg == NULL)
-			continue;
-		flush(fswritefd, &cgbufs[cnt]);
-		free(cgbufs[cnt].b_un.b_cg);
+	if (cgbufs != NULL) {
+		for (cnt = 0; cnt < sblock.fs_ncg; cnt++) {
+			if (cgbufs[cnt].b_un.b_cg == NULL)
+				continue;
+			flush(fswritefd, &cgbufs[cnt]);
+			free(cgbufs[cnt].b_un.b_cg);
+		}
+		free(cgbufs);
 	}
-	free(cgbufs);
 	pbp = pdirbp = (struct bufarea *)0;
 	if (cursnapshot == 0 && sblock.fs_clean != markclean) {
 		if ((sblock.fs_clean = markclean) != 0) {
_______________________________________________
svn-src-all@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscribe@freebsd.org"

Comment 5 Kirk McKusick freebsd_committer

2014-03-12 01:33:38 UTC

State Changed
From-To: open->patched

Suggested fix applied to HEAD; if no problems reported, it will be 
MFC'ed to 9 & 10 in a week.

Comment 6 dfilter service freebsd_committer

2014-03-22 11:43:45 UTC

Author: mckusick
Date: Sat Mar 22 11:43:35 2014
New Revision: 263629
URL: http://svnweb.freebsd.org/changeset/base/263629

Log:
  MFC of 263062:
  
  Avoid segment fault when attempting to clean up cylinder group
  buffer cache.
  
  PR:             187221
  Submitted by:   Petr Lampa <lampa@fit.vutbr.cz>
  Obtained from:  Petr Lampa <lampa@fit.vutbr.cz>
  MFC after:      1 week
  
  MFC of 262488:
  
  Arguments for malloc and calloc should be size_t, not int.
  Use proper bounds check when trying to free cached memory.
  
  Spotted by: Xin Li
  Tested by:  Dmitry Sivachenko
  MFC after:  2 weeks

Modified:
  stable/10/sbin/fsck_ffs/fsck.h
  stable/10/sbin/fsck_ffs/fsutil.c
Directory Properties:
  stable/10/   (props changed)

Modified: stable/10/sbin/fsck_ffs/fsck.h
==============================================================================
--- stable/10/sbin/fsck_ffs/fsck.h	Sat Mar 22 11:26:39 2014	(r263628)
+++ stable/10/sbin/fsck_ffs/fsck.h	Sat Mar 22 11:43:35 2014	(r263629)
@@ -369,7 +369,7 @@ int flushentry(void);
  * to get space.
  */
 static inline void*
-Malloc(int size)
+Malloc(size_t size)
 {
 	void *retval;
 
@@ -384,7 +384,7 @@ Malloc(int size)
  * to get space.
  */
 static inline void*
-Calloc(int cnt, int size)
+Calloc(size_t cnt, size_t size)
 {
 	void *retval;
 

Modified: stable/10/sbin/fsck_ffs/fsutil.c
==============================================================================
--- stable/10/sbin/fsck_ffs/fsutil.c	Sat Mar 22 11:26:39 2014	(r263628)
+++ stable/10/sbin/fsck_ffs/fsutil.c	Sat Mar 22 11:43:35 2014	(r263629)
@@ -225,7 +225,7 @@ cgget(int cg)
 	struct cg *cgp;
 
 	if (cgbufs == NULL) {
-		cgbufs = Calloc(sblock.fs_ncg, sizeof(struct bufarea));
+		cgbufs = calloc(sblock.fs_ncg, sizeof(struct bufarea));
 		if (cgbufs == NULL)
 			errx(EEXIT, "cannot allocate cylinder group buffers");
 	}
@@ -254,6 +254,8 @@ flushentry(void)
 {
 	struct bufarea *cgbp;
 
+	if (flushtries == sblock.fs_ncg || cgbufs == NULL)
+		return (0);
 	cgbp = &cgbufs[flushtries++];
 	if (cgbp->b_un.b_cg == NULL)
 		return (0);
@@ -434,13 +436,15 @@ ckfini(int markclean)
 	}
 	if (numbufs != cnt)
 		errx(EEXIT, "panic: lost %d buffers", numbufs - cnt);
-	for (cnt = 0; cnt < sblock.fs_ncg; cnt++) {
-		if (cgbufs[cnt].b_un.b_cg == NULL)
-			continue;
-		flush(fswritefd, &cgbufs[cnt]);
-		free(cgbufs[cnt].b_un.b_cg);
+	if (cgbufs != NULL) {
+		for (cnt = 0; cnt < sblock.fs_ncg; cnt++) {
+			if (cgbufs[cnt].b_un.b_cg == NULL)
+				continue;
+			flush(fswritefd, &cgbufs[cnt]);
+			free(cgbufs[cnt].b_un.b_cg);
+		}
+		free(cgbufs);
 	}
-	free(cgbufs);
 	pbp = pdirbp = (struct bufarea *)0;
 	if (cursnapshot == 0 && sblock.fs_clean != markclean) {
 		if ((sblock.fs_clean = markclean) != 0) {
_______________________________________________
svn-src-all@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscribe@freebsd.org"

Comment 7 dfilter service freebsd_committer

2014-03-22 11:49:49 UTC

Author: mckusick
Date: Sat Mar 22 11:49:44 2014
New Revision: 263630
URL: http://svnweb.freebsd.org/changeset/base/263630

Log:
  MFC of 263062:
  
  Avoid segment fault when attempting to clean up cylinder group
  buffer cache.
  
  PR:             187221
  Submitted by:   Petr Lampa <lampa@fit.vutbr.cz>
  Obtained from:  Petr Lampa <lampa@fit.vutbr.cz>
  MFC after:      1 week
  
  MFC of 262488:
  
  Arguments for malloc and calloc should be size_t, not int.
  Use proper bounds check when trying to free cached memory.
  
  Spotted by: Xin Li
  Tested by:  Dmitry Sivachenko
  MFC after:  2 weeks

Modified:
  stable/9/sbin/fsck_ffs/fsck.h
  stable/9/sbin/fsck_ffs/fsutil.c
Directory Properties:
  stable/9/   (props changed)
  stable/9/sbin/   (props changed)
  stable/9/sbin/fsck_ffs/   (props changed)
  stable/9/sbin/ggate/   (props changed)

Modified: stable/9/sbin/fsck_ffs/fsck.h
==============================================================================
--- stable/9/sbin/fsck_ffs/fsck.h	Sat Mar 22 11:43:35 2014	(r263629)
+++ stable/9/sbin/fsck_ffs/fsck.h	Sat Mar 22 11:49:44 2014	(r263630)
@@ -366,7 +366,7 @@ int flushentry(void);
  * to get space.
  */
 static inline void*
-Malloc(int size)
+Malloc(size_t size)
 {
 	void *retval;
 
@@ -381,7 +381,7 @@ Malloc(int size)
  * to get space.
  */
 static inline void*
-Calloc(int cnt, int size)
+Calloc(size_t cnt, size_t size)
 {
 	void *retval;
 

Modified: stable/9/sbin/fsck_ffs/fsutil.c
==============================================================================
--- stable/9/sbin/fsck_ffs/fsutil.c	Sat Mar 22 11:43:35 2014	(r263629)
+++ stable/9/sbin/fsck_ffs/fsutil.c	Sat Mar 22 11:49:44 2014	(r263630)
@@ -205,7 +205,7 @@ cgget(int cg)
 	struct cg *cgp;
 
 	if (cgbufs == NULL) {
-		cgbufs = Calloc(sblock.fs_ncg, sizeof(struct bufarea));
+		cgbufs = calloc(sblock.fs_ncg, sizeof(struct bufarea));
 		if (cgbufs == NULL)
 			errx(EEXIT, "cannot allocate cylinder group buffers");
 	}
@@ -234,6 +234,8 @@ flushentry(void)
 {
 	struct bufarea *cgbp;
 
+	if (flushtries == sblock.fs_ncg || cgbufs == NULL)
+		return (0);
 	cgbp = &cgbufs[flushtries++];
 	if (cgbp->b_un.b_cg == NULL)
 		return (0);
@@ -414,13 +416,15 @@ ckfini(int markclean)
 	}
 	if (numbufs != cnt)
 		errx(EEXIT, "panic: lost %d buffers", numbufs - cnt);
-	for (cnt = 0; cnt < sblock.fs_ncg; cnt++) {
-		if (cgbufs[cnt].b_un.b_cg == NULL)
-			continue;
-		flush(fswritefd, &cgbufs[cnt]);
-		free(cgbufs[cnt].b_un.b_cg);
+	if (cgbufs != NULL) {
+		for (cnt = 0; cnt < sblock.fs_ncg; cnt++) {
+			if (cgbufs[cnt].b_un.b_cg == NULL)
+				continue;
+			flush(fswritefd, &cgbufs[cnt]);
+			free(cgbufs[cnt].b_un.b_cg);
+		}
+		free(cgbufs);
 	}
-	free(cgbufs);
 	pbp = pdirbp = (struct bufarea *)0;
 	if (cursnapshot == 0 && sblock.fs_clean != markclean) {
 		if ((sblock.fs_clean = markclean) != 0) {
_______________________________________________
svn-src-all@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscribe@freebsd.org"

Comment 8 Kirk McKusick freebsd_committer

2014-03-22 14:37:53 UTC

State Changed
From-To: patched->closed

The fix has been MFC'ed to 9-stable and 10-stable.