f2fs: fix use-after-free of dicard command entry

As Dan Carpenter reported: The patch 20ee438: "f2fs: issue small discard by LBA order" from Jul 8, 2018, leads to the following Smatch warning: fs/f2fs/segment.c:1277 __issue_discard_cmd_orderly() warn: 'dc' was already freed. See also: fs/f2fs/segment.c:2550 __issue_discard_cmd_range() warn: 'dc' was already freed. In order to fix this issue, let's get error from __submit_discard_cmd(), and release current discard command after we referenced next one. Reported-by: Dan Carpenter <[email protected]> Signed-off-by: Chao Yu <[email protected]> Signed-off-by: Jaegeuk Kim <[email protected]>
fracktal · Aug 13, 2018 · 6b9cb12 · 6b9cb12
1 parent b83dcfe
commit 6b9cb12
Showing 1 changed file with 45 additions and 34 deletions.
diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
@@ -994,7 +994,7 @@ static void __update_discard_tree_range(struct f2fs_sb_info *sbi,
 				struct block_device *bdev, block_t lstart,
 				block_t start, block_t len);
 /* this function is copied from blkdev_issue_discard from block/blk-lib.c */
-static void __submit_discard_cmd(struct f2fs_sb_info *sbi,
+static int __submit_discard_cmd(struct f2fs_sb_info *sbi,
 						struct discard_policy *dpolicy,
 						struct discard_cmd *dc,
 						unsigned int *issued)
@@ -1011,10 +1011,10 @@ static void __submit_discard_cmd(struct f2fs_sb_info *sbi,
 	int err = 0;
 
 	if (dc->state != D_PREP)
-		return;
+		return 0;
 
 	if (is_sbi_flag_set(sbi, SBI_NEED_FSCK))
-		return;
+		return 0;
 
 	trace_f2fs_issue_discard(bdev, dc->start, dc->len);
 
@@ -1053,52 +1053,54 @@ static void __submit_discard_cmd(struct f2fs_sb_info *sbi,
 					SECTOR_FROM_BLOCK(len),
 					GFP_NOFS, 0, &bio);
 submit:
-		if (!err && bio) {
-			/*
-			 * should keep before submission to avoid D_DONE
-			 * right away
-			 */
+		if (err) {
 			spin_lock_irqsave(&dc->lock, flags);
-			if (last)
+			if (dc->state == D_PARTIAL)
 				dc->state = D_SUBMIT;
-			else
-				dc->state = D_PARTIAL;
-			dc->bio_ref++;
 			spin_unlock_irqrestore(&dc->lock, flags);
 
-			atomic_inc(&dcc->issing_discard);
-			dc->issuing++;
-			list_move_tail(&dc->list, wait_list);
+			break;
+		}
 
-			/* sanity check on discard range */
-			__check_sit_bitmap(sbi, start, start + len);
+		f2fs_bug_on(sbi, !bio);
 
-			bio->bi_private = dc;
-			bio->bi_end_io = f2fs_submit_discard_endio;
-			bio->bi_opf |= flag;
-			submit_bio(bio);
+		/*
+		 * should keep before submission to avoid D_DONE
+		 * right away
+		 */
+		spin_lock_irqsave(&dc->lock, flags);
+		if (last)
+			dc->state = D_SUBMIT;
+		else
+			dc->state = D_PARTIAL;
+		dc->bio_ref++;
+		spin_unlock_irqrestore(&dc->lock, flags);
 
-			atomic_inc(&dcc->issued_discard);
+		atomic_inc(&dcc->issing_discard);
+		dc->issuing++;
+		list_move_tail(&dc->list, wait_list);
 
-			f2fs_update_iostat(sbi, FS_DISCARD, 1);
-		} else {
-			spin_lock_irqsave(&dc->lock, flags);
-			if (dc->state == D_PARTIAL)
-				dc->state = D_SUBMIT;
-			spin_unlock_irqrestore(&dc->lock, flags);
+		/* sanity check on discard range */
+		__check_sit_bitmap(sbi, start, start + len);
 
-			__remove_discard_cmd(sbi, dc);
-			err = -EIO;
-		}
+		bio->bi_private = dc;
+		bio->bi_end_io = f2fs_submit_discard_endio;
+		bio->bi_opf |= flag;
+		submit_bio(bio);
+
+		atomic_inc(&dcc->issued_discard);
+
+		f2fs_update_iostat(sbi, FS_DISCARD, 1);
 
 		lstart += len;
 		start += len;
 		total_len -= len;
 		len = total_len;
 	}
 
-	if (len)
+	if (!err && len)
 		__update_discard_tree_range(sbi, bdev, lstart, start, len);
+	return err;
 }
 
 static struct discard_cmd *__insert_discard_tree(struct f2fs_sb_info *sbi,
@@ -1304,6 +1306,7 @@ static unsigned int __issue_discard_cmd_orderly(struct f2fs_sb_info *sbi,
 
 	while (dc) {
 		struct rb_node *node;
+		int err = 0;
 
 		if (dc->state != D_PREP)
 			goto next;
@@ -1314,12 +1317,14 @@ static unsigned int __issue_discard_cmd_orderly(struct f2fs_sb_info *sbi,
 		}
 
 		dcc->next_pos = dc->lstart + dc->len;
-		__submit_discard_cmd(sbi, dpolicy, dc, &issued);
+		err = __submit_discard_cmd(sbi, dpolicy, dc, &issued);
 
 		if (issued >= dpolicy->max_requests)
 			break;
 next:
 		node = rb_next(&dc->rb_node);
+		if (err)
+			__remove_discard_cmd(sbi, dc);
 		dc = rb_entry_safe(node, struct discard_cmd, rb_node);
 	}
 
@@ -2571,6 +2576,7 @@ static unsigned int __issue_discard_cmd_range(struct f2fs_sb_info *sbi,
 
 	while (dc && dc->lstart <= end) {
 		struct rb_node *node;
+		int err = 0;
 
 		if (dc->len < dpolicy->granularity)
 			goto skip;
@@ -2580,11 +2586,14 @@ static unsigned int __issue_discard_cmd_range(struct f2fs_sb_info *sbi,
 			goto skip;
 		}
 
-		__submit_discard_cmd(sbi, dpolicy, dc, &issued);
+		err = __submit_discard_cmd(sbi, dpolicy, dc, &issued);
 
 		if (issued >= dpolicy->max_requests) {
 			start = dc->lstart + dc->len;
 
+			if (err)
+				__remove_discard_cmd(sbi, dc);
+
 			blk_finish_plug(&plug);
 			mutex_unlock(&dcc->cmd_lock);
 			trimmed += __wait_all_discard_cmd(sbi, NULL);
@@ -2593,6 +2602,8 @@ static unsigned int __issue_discard_cmd_range(struct f2fs_sb_info *sbi,
 		}
 skip:
 		node = rb_next(&dc->rb_node);
+		if (err)
+			__remove_discard_cmd(sbi, dc);
 		dc = rb_entry_safe(node, struct discard_cmd, rb_node);
 
 		if (fatal_signal_pending(current))