gfs2-utils: master - gfs2_edit: Fix use-after-free in find_wrap_pt

[Andrew Price]

Gitweb:        http://git.fedorahosted.org/git/?p=gfs2-utils.git;a=commitdiff;h=f67b54c097b...
Commit:        f67b54c097b5fbb6ace1c3589139c35e3d46adf1
Parent:        2b4fcf7ac3589fa8cef3b9985fdca3562b0fbb07
Author:        Andrew Price 
AuthorDate:    Thu Sep 18 17:43:47 2014 +0100
Committer:     Andrew Price 
CommitterDate: Thu Sep 18 17:54:59 2014 +0100

gfs2_edit: Fix use-after-free in find_wrap_pt

Spotted by coverity: Calling "brelse(struct gfs2_buffer_head *)"
dereferences freed pointer.

Refactor the find_wrap_pt function to make it easier to manage memory
and minimise scope.

Signed-off-by: Andrew Price 
---
 gfs2/edit/journal.c |   70 ++++++++++++++++++++++++++++----------------------
 1 files changed, 39 insertions(+), 31 deletions(-)

diff --git a/gfs2/edit/journal.c b/gfs2/edit/journal.c
index 58f4798..81d7a90 100644
--- a/gfs2/edit/journal.c
+++ b/gfs2/edit/journal.c
@@ -297,50 +297,58 @@ static int print_ld_blks(const uint64_t *b, const char *end, int start_line,
    return bcount;
 }

+static int is_wrap_pt(char *buf, uint64_t *highest_seq)
+{
+	struct gfs2_buffer_head tbh = { .b_data = buf };
+
+	if (get_block_type(&tbh, NULL) == GFS2_METATYPE_LH) {
+		uint64_t seq;
+
+		if (sbd.gfs1) {
+			struct gfs_log_header lh;
+			gfs_log_header_in(&lh, &tbh);
+			seq = lh.lh_sequence;
+		} else {
+			struct gfs2_log_header lh;
+			gfs2_log_header_in(&lh, &tbh);
+			seq = lh.lh_sequence;
+		}
+		if (seq < *highest_seq)
+			return 1;
+		*highest_seq = seq;
+	}
+	return 0;
+}
+
 /**
  * find_wrap_pt - figure out where a journal wraps
  * Returns: The wrap point, in bytes
  */
-static uint64_t find_wrap_pt(struct gfs2_inode *j_inode, char *jbuf,
-			     uint64_t jblock, uint64_t j_size)
+static uint64_t find_wrap_pt(struct gfs2_inode *ji, char *jbuf, uint64_t jblock, uint64_t j_size)
 {
-	struct gfs2_buffer_head *j_bh = NULL, dummy_bh;
-	uint64_t jb, abs_block;
-	int error;
+	uint64_t jb = 0;
    uint64_t highest_seq = 0;

for (jb = 0; jb < j_size; jb += (sbd.gfs1 ? 1 : sbd.bsize)) {
+		int found = 0;
+
    	if (sbd.gfs1) {
-			if (j_bh)
-				brelse(j_bh);
+			struct gfs2_buffer_head *j_bh;
+
    		j_bh = bread(&sbd, jblock + jb);
-			abs_block = jblock + jb;
-			dummy_bh.b_data = j_bh->b_data;
+			found = is_wrap_pt(j_bh->b_data, &highest_seq);
+			brelse(j_bh);
    	} else {
-			error = fsck_readi(j_inode, (void *)jbuf, jb,
-					   sbd.bsize, &abs_block);
-			if (!error) /* end of file */
-				break;
-			dummy_bh.b_data = jbuf;
-		}
-		if (get_block_type(&dummy_bh, NULL) == GFS2_METATYPE_LH) {
-			struct gfs2_log_header lh;
-			struct gfs_log_header lh1;
+			int copied;
+			uint64_t abs_block;

-			if (sbd.gfs1) {
-				gfs_log_header_in(&lh1, &dummy_bh);
-				if (lh1.lh_sequence < highest_seq)
-					return jb;
-				highest_seq = lh1.lh_sequence;
-			} else {
-				gfs2_log_header_in(&lh, &dummy_bh);
-				if (lh.lh_sequence < highest_seq)
-					return jb;
-				highest_seq = lh.lh_sequence;
-			}
+			copied = fsck_readi(ji, jbuf, jb, sbd.bsize, &abs_block);
+			if (!copied) /* end of file */
+				break;
+			found = is_wrap_pt(jbuf, &highest_seq);
    	}
-		if (j_bh)
-			brelse(j_bh);
+		if (found)
+			return jb;
    }
    return 0;
 }