aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorDaeho Jeong2020-12-04 22:26:26 -0600
committerJaegeuk Kim2020-12-21 15:33:19 -0600
commitd4ea8d8b67c4ad489d92ffa53b4681679243b885 (patch)
treeab0557c0a8f022436a84446e38eedf93575c797a
parentb91c6825c376eeebf1f4ecfa61e7cc0300addc9c (diff)
downloadkernel-d4ea8d8b67c4ad489d92ffa53b4681679243b885.tar.gz
kernel-d4ea8d8b67c4ad489d92ffa53b4681679243b885.tar.xz
kernel-d4ea8d8b67c4ad489d92ffa53b4681679243b885.zip
f2fs: fix race of pending_pages in decompression
I found out f2fs_free_dic() is invoked in a wrong timing, but f2fs_verify_bio() still needed the dic info and it triggered the below kernel panic. It has been caused by the race condition of pending_pages value between decompression and verity logic, when the same compression cluster had been split in different bios. By split bios, f2fs_verify_bio() ended up with decreasing pending_pages value before it is reset to nr_cpages by f2fs_decompress_pages() and caused the kernel panic. [ 4416.564763] Unable to handle kernel NULL pointer dereference at virtual address 0000000000000000 ... [ 4416.896016] Workqueue: fsverity_read_queue f2fs_verity_work [ 4416.908515] pc : fsverity_verify_page+0x20/0x78 [ 4416.913721] lr : f2fs_verify_bio+0x11c/0x29c [ 4416.913722] sp : ffffffc019533cd0 [ 4416.913723] x29: ffffffc019533cd0 x28: 0000000000000402 [ 4416.913724] x27: 0000000000000001 x26: 0000000000000100 [ 4416.913726] x25: 0000000000000001 x24: 0000000000000004 [ 4416.913727] x23: 0000000000001000 x22: 0000000000000000 [ 4416.913728] x21: 0000000000000000 x20: ffffffff2076f9c0 [ 4416.913729] x19: ffffffff2076f9c0 x18: ffffff8a32380c30 [ 4416.913731] x17: ffffffc01f966d97 x16: 0000000000000298 [ 4416.913732] x15: 0000000000000000 x14: 0000000000000000 [ 4416.913733] x13: f074faec89ffffff x12: 0000000000000000 [ 4416.913734] x11: 0000000000001000 x10: 0000000000001000 [ 4416.929176] x9 : ffffffff20d1f5c7 x8 : 0000000000000000 [ 4416.929178] x7 : 626d7464ff286b6b x6 : ffffffc019533ade [ 4416.929179] x5 : 000000008049000e x4 : ffffffff2793e9e0 [ 4416.929180] x3 : 000000008049000e x2 : ffffff89ecfa74d0 [ 4416.929181] x1 : 0000000000000c40 x0 : ffffffff2076f9c0 [ 4416.929184] Call trace: [ 4416.929187] fsverity_verify_page+0x20/0x78 [ 4416.929189] f2fs_verify_bio+0x11c/0x29c [ 4416.929192] f2fs_verity_work+0x58/0x84 [ 4417.050667] process_one_work+0x270/0x47c [ 4417.055354] worker_thread+0x27c/0x4d8 [ 4417.059784] kthread+0x13c/0x320 [ 4417.063693] ret_from_fork+0x10/0x18 Chao pointed this can happen by the below race condition. Thread A f2fs_post_read_wq fsverity_wq - f2fs_read_multi_pages() - f2fs_alloc_dic - dic->pending_pages = 2 - submit_bio() - submit_bio() - f2fs_post_read_work() handle first bio - f2fs_decompress_work() - __read_end_io() - f2fs_decompress_pages() - dic->pending_pages-- - enqueue f2fs_verity_work() - f2fs_verity_work() handle first bio - f2fs_verify_bio() - dic->pending_pages-- - f2fs_post_read_work() handle second bio - f2fs_decompress_work() - enqueue f2fs_verity_work() - f2fs_verify_pages() - f2fs_free_dic() - f2fs_verity_work() handle second bio - f2fs_verfy_bio() - use-after-free on dic Signed-off-by: Daeho Jeong <daehojeong@google.com> Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org>
-rw-r--r--fs/f2fs/compress.c2
-rw-r--r--fs/f2fs/data.c58
-rw-r--r--fs/f2fs/f2fs.h1
3 files changed, 48 insertions, 13 deletions
diff --git a/fs/f2fs/compress.c b/fs/f2fs/compress.c
index d23bebb6ccd3..f05d409fd0ed 100644
--- a/fs/f2fs/compress.c
+++ b/fs/f2fs/compress.c
@@ -821,8 +821,6 @@ destroy_decompress_ctx:
821 if (cops->destroy_decompress_ctx) 821 if (cops->destroy_decompress_ctx)
822 cops->destroy_decompress_ctx(dic); 822 cops->destroy_decompress_ctx(dic);
823out_free_dic: 823out_free_dic:
824 if (verity)
825 atomic_set(&dic->pending_pages, dic->nr_cpages);
826 if (!verity) 824 if (!verity)
827 f2fs_decompress_end_io(dic->rpages, dic->cluster_size, 825 f2fs_decompress_end_io(dic->rpages, dic->cluster_size,
828 ret, false); 826 ret, false);
diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
index cb28089e1eff..aa34d620bec9 100644
--- a/fs/f2fs/data.c
+++ b/fs/f2fs/data.c
@@ -202,7 +202,7 @@ static void f2fs_verify_bio(struct bio *bio)
202 dic = (struct decompress_io_ctx *)page_private(page); 202 dic = (struct decompress_io_ctx *)page_private(page);
203 203
204 if (dic) { 204 if (dic) {
205 if (atomic_dec_return(&dic->pending_pages)) 205 if (atomic_dec_return(&dic->verity_pages))
206 continue; 206 continue;
207 f2fs_verify_pages(dic->rpages, 207 f2fs_verify_pages(dic->rpages,
208 dic->cluster_size); 208 dic->cluster_size);
@@ -1030,7 +1030,8 @@ static inline bool f2fs_need_verity(const struct inode *inode, pgoff_t idx)
1030 1030
1031static struct bio *f2fs_grab_read_bio(struct inode *inode, block_t blkaddr, 1031static struct bio *f2fs_grab_read_bio(struct inode *inode, block_t blkaddr,
1032 unsigned nr_pages, unsigned op_flag, 1032 unsigned nr_pages, unsigned op_flag,
1033 pgoff_t first_idx, bool for_write) 1033 pgoff_t first_idx, bool for_write,
1034 bool for_verity)
1034{ 1035{
1035 struct f2fs_sb_info *sbi = F2FS_I_SB(inode); 1036 struct f2fs_sb_info *sbi = F2FS_I_SB(inode);
1036 struct bio *bio; 1037 struct bio *bio;
@@ -1052,7 +1053,7 @@ static struct bio *f2fs_grab_read_bio(struct inode *inode, block_t blkaddr,
1052 post_read_steps |= 1 << STEP_DECRYPT; 1053 post_read_steps |= 1 << STEP_DECRYPT;
1053 if (f2fs_compressed_file(inode)) 1054 if (f2fs_compressed_file(inode))
1054 post_read_steps |= 1 << STEP_DECOMPRESS_NOWQ; 1055 post_read_steps |= 1 << STEP_DECOMPRESS_NOWQ;
1055 if (f2fs_need_verity(inode, first_idx)) 1056 if (for_verity && f2fs_need_verity(inode, first_idx))
1056 post_read_steps |= 1 << STEP_VERITY; 1057 post_read_steps |= 1 << STEP_VERITY;
1057 1058
1058 if (post_read_steps) { 1059 if (post_read_steps) {
@@ -1082,7 +1083,7 @@ static int f2fs_submit_page_read(struct inode *inode, struct page *page,
1082 struct bio *bio; 1083 struct bio *bio;
1083 1084
1084 bio = f2fs_grab_read_bio(inode, blkaddr, 1, op_flags, 1085 bio = f2fs_grab_read_bio(inode, blkaddr, 1, op_flags,
1085 page->index, for_write); 1086 page->index, for_write, true);
1086 if (IS_ERR(bio)) 1087 if (IS_ERR(bio))
1087 return PTR_ERR(bio); 1088 return PTR_ERR(bio);
1088 1089
@@ -2120,7 +2121,7 @@ submit_and_realloc:
2120 if (bio == NULL) { 2121 if (bio == NULL) {
2121 bio = f2fs_grab_read_bio(inode, block_nr, nr_pages, 2122 bio = f2fs_grab_read_bio(inode, block_nr, nr_pages,
2122 is_readahead ? REQ_RAHEAD : 0, page->index, 2123 is_readahead ? REQ_RAHEAD : 0, page->index,
2123 false); 2124 false, true);
2124 if (IS_ERR(bio)) { 2125 if (IS_ERR(bio)) {
2125 ret = PTR_ERR(bio); 2126 ret = PTR_ERR(bio);
2126 bio = NULL; 2127 bio = NULL;
@@ -2166,6 +2167,8 @@ int f2fs_read_multi_pages(struct compress_ctx *cc, struct bio **bio_ret,
2166 sector_t last_block_in_file; 2167 sector_t last_block_in_file;
2167 const unsigned blocksize = blks_to_bytes(inode, 1); 2168 const unsigned blocksize = blks_to_bytes(inode, 1);
2168 struct decompress_io_ctx *dic = NULL; 2169 struct decompress_io_ctx *dic = NULL;
2170 struct bio_post_read_ctx *ctx;
2171 bool for_verity = false;
2169 int i; 2172 int i;
2170 int ret = 0; 2173 int ret = 0;
2171 2174
@@ -2231,10 +2234,29 @@ int f2fs_read_multi_pages(struct compress_ctx *cc, struct bio **bio_ret,
2231 goto out_put_dnode; 2234 goto out_put_dnode;
2232 } 2235 }
2233 2236
2237 /*
2238 * It's possible to enable fsverity on the fly when handling a cluster,
2239 * which requires complicated error handling. Instead of adding more
2240 * complexity, let's give a rule where end_io post-processes fsverity
2241 * per cluster. In order to do that, we need to submit bio, if previous
2242 * bio sets a different post-process policy.
2243 */
2244 if (fsverity_active(cc->inode)) {
2245 atomic_set(&dic->verity_pages, cc->nr_cpages);
2246 for_verity = true;
2247
2248 if (bio) {
2249 ctx = bio->bi_private;
2250 if (!(ctx->enabled_steps & (1 << STEP_VERITY))) {
2251 __submit_bio(sbi, bio, DATA);
2252 bio = NULL;
2253 }
2254 }
2255 }
2256
2234 for (i = 0; i < dic->nr_cpages; i++) { 2257 for (i = 0; i < dic->nr_cpages; i++) {
2235 struct page *page = dic->cpages[i]; 2258 struct page *page = dic->cpages[i];
2236 block_t blkaddr; 2259 block_t blkaddr;
2237 struct bio_post_read_ctx *ctx;
2238 2260
2239 blkaddr = data_blkaddr(dn.inode, dn.node_page, 2261 blkaddr = data_blkaddr(dn.inode, dn.node_page,
2240 dn.ofs_in_node + i + 1); 2262 dn.ofs_in_node + i + 1);
@@ -2250,17 +2272,31 @@ submit_and_realloc:
2250 if (!bio) { 2272 if (!bio) {
2251 bio = f2fs_grab_read_bio(inode, blkaddr, nr_pages, 2273 bio = f2fs_grab_read_bio(inode, blkaddr, nr_pages,
2252 is_readahead ? REQ_RAHEAD : 0, 2274 is_readahead ? REQ_RAHEAD : 0,
2253 page->index, for_write); 2275 page->index, for_write, for_verity);
2254 if (IS_ERR(bio)) { 2276 if (IS_ERR(bio)) {
2277 unsigned int remained = dic->nr_cpages - i;
2278 bool release = false;
2279
2255 ret = PTR_ERR(bio); 2280 ret = PTR_ERR(bio);
2256 dic->failed = true; 2281 dic->failed = true;
2257 if (!atomic_sub_return(dic->nr_cpages - i, 2282
2258 &dic->pending_pages)) { 2283 if (for_verity) {
2284 if (!atomic_sub_return(remained,
2285 &dic->verity_pages))
2286 release = true;
2287 } else {
2288 if (!atomic_sub_return(remained,
2289 &dic->pending_pages))
2290 release = true;
2291 }
2292
2293 if (release) {
2259 f2fs_decompress_end_io(dic->rpages, 2294 f2fs_decompress_end_io(dic->rpages,
2260 cc->cluster_size, true, 2295 cc->cluster_size, true,
2261 false); 2296 false);
2262 f2fs_free_dic(dic); 2297 f2fs_free_dic(dic);
2263 } 2298 }
2299
2264 f2fs_put_dnode(&dn); 2300 f2fs_put_dnode(&dn);
2265 *bio_ret = NULL; 2301 *bio_ret = NULL;
2266 return ret; 2302 return ret;
diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
index 594df6391390..7364d453783f 100644
--- a/fs/f2fs/f2fs.h
+++ b/fs/f2fs/f2fs.h
@@ -1354,6 +1354,7 @@ struct decompress_io_ctx {
1354 size_t rlen; /* valid data length in rbuf */ 1354 size_t rlen; /* valid data length in rbuf */
1355 size_t clen; /* valid data length in cbuf */ 1355 size_t clen; /* valid data length in cbuf */
1356 atomic_t pending_pages; /* in-flight compressed page count */ 1356 atomic_t pending_pages; /* in-flight compressed page count */
1357 atomic_t verity_pages; /* in-flight page count for verity */
1357 bool failed; /* indicate IO error during decompression */ 1358 bool failed; /* indicate IO error during decompression */
1358 void *private; /* payload buffer for specified decompression algorithm */ 1359 void *private; /* payload buffer for specified decompression algorithm */
1359 void *private2; /* extra payload buffer */ 1360 void *private2; /* extra payload buffer */