From fe5f56b955b2bec02ab82e98efaea7b496853ad8 Mon Sep 17 00:00:00 2001 From: Jim Huang Date: Mon, 24 Nov 2025 17:14:30 +0800 Subject: [PATCH] Fix high-severity bugs Bug fixes: - bitmap.h: Return 0 instead of -EIO on sb_bread failure (type mismatch) Also restore bitmap and block count on allocation failure - inode.c: Fix buffer leak in simplefs_lookup() - release bh on error - inode.c: Initialize fi variable and use explicit indices in simplefs_set_file_into_dir() to prevent undefined behavior - inode.c: Add null termination after strncpy for filenames - inode.c: Add bounds validation for extent avail index in simplefs_create(), simplefs_link(), and simplefs_symlink() - inode.c: Add missing extent nr_files increment in link/symlink In addition, this commit adds Linux 6.15+ compatibility. - Update write_begin() signature (kiocb-based API) - Update write_end() signature and file reference access - Conditionally exclude writepage from aops (deprecated) - Update simplefs_mkdir() to return 'struct dentry *' --- bitmap.h | 4 +- file.c | 113 ++++++++++++++++++++++++++++++++++++++++++++++++------- inode.c | 71 ++++++++++++++++++++++++++-------- 3 files changed, 158 insertions(+), 30 deletions(-) diff --git a/bitmap.h b/bitmap.h index 8a317d1..09606a7 100644 --- a/bitmap.h +++ b/bitmap.h @@ -57,8 +57,10 @@ static inline uint32_t get_free_blocks(struct super_block *sb, uint32_t len) bh = sb_bread(sb, ret + i); if (!bh) { pr_err("get_free_blocks: sb_bread failed for block %d\n", ret + i); + /* Restore all len blocks - bitmap was cleared atomically */ + bitmap_set(sbi->bfree_bitmap, ret, len); sbi->nr_free_blocks += len; - return -EIO; + return 0; /* Return 0 to indicate failure (0 is reserved) */ } memset(bh->b_data, 0, SIMPLEFS_BLOCK_SIZE); mark_buffer_dirty(bh); diff --git a/file.c b/file.c index 5987681..24a2d22 100644 --- a/file.c +++ b/file.c @@ -113,13 +113,63 @@ static int simplefs_writepage(struct page *page, struct writeback_control *wbc) * the data into the page cache. This function checks if the write operation * can complete and allocates the necessary blocks through block_write_begin(). */ -#if SIMPLEFS_AT_LEAST(6, 12, 0) +#if SIMPLEFS_AT_LEAST(6, 15, 0) +static int simplefs_write_begin(const struct kiocb *iocb, + struct address_space *mapping, + loff_t pos, + unsigned int len, + struct folio **foliop, + void **fsdata) +{ + struct file *file = iocb->ki_filp; + struct simplefs_sb_info *sbi = SIMPLEFS_SB(file->f_inode->i_sb); + int err; + uint32_t nr_allocs = 0; + + if (pos + len > SIMPLEFS_MAX_FILESIZE) + return -ENOSPC; + + nr_allocs = max(pos + len, file->f_inode->i_size) / SIMPLEFS_BLOCK_SIZE; + if (nr_allocs > file->f_inode->i_blocks - 1) + nr_allocs -= file->f_inode->i_blocks - 1; + else + nr_allocs = 0; + if (nr_allocs > sbi->nr_free_blocks) + return -ENOSPC; + + err = block_write_begin(mapping, pos, len, foliop, simplefs_file_get_block); + if (err < 0) + pr_err("newly allocated blocks reclaim not implemented yet\n"); + return err; +} +#elif SIMPLEFS_AT_LEAST(6, 12, 0) static int simplefs_write_begin(struct file *file, struct address_space *mapping, loff_t pos, unsigned int len, struct folio **foliop, void **fsdata) +{ + struct simplefs_sb_info *sbi = SIMPLEFS_SB(file->f_inode->i_sb); + int err; + uint32_t nr_allocs = 0; + + if (pos + len > SIMPLEFS_MAX_FILESIZE) + return -ENOSPC; + + nr_allocs = max(pos + len, file->f_inode->i_size) / SIMPLEFS_BLOCK_SIZE; + if (nr_allocs > file->f_inode->i_blocks - 1) + nr_allocs -= file->f_inode->i_blocks - 1; + else + nr_allocs = 0; + if (nr_allocs > sbi->nr_free_blocks) + return -ENOSPC; + + err = block_write_begin(mapping, pos, len, foliop, simplefs_file_get_block); + if (err < 0) + pr_err("newly allocated blocks reclaim not implemented yet\n"); + return err; +} #elif SIMPLEFS_AT_LEAST(5, 19, 0) static int simplefs_write_begin(struct file *file, struct address_space *mapping, @@ -127,6 +177,27 @@ static int simplefs_write_begin(struct file *file, unsigned int len, struct page **pagep, void **fsdata) +{ + struct simplefs_sb_info *sbi = SIMPLEFS_SB(file->f_inode->i_sb); + int err; + uint32_t nr_allocs = 0; + + if (pos + len > SIMPLEFS_MAX_FILESIZE) + return -ENOSPC; + + nr_allocs = max(pos + len, file->f_inode->i_size) / SIMPLEFS_BLOCK_SIZE; + if (nr_allocs > file->f_inode->i_blocks - 1) + nr_allocs -= file->f_inode->i_blocks - 1; + else + nr_allocs = 0; + if (nr_allocs > sbi->nr_free_blocks) + return -ENOSPC; + + err = block_write_begin(mapping, pos, len, pagep, simplefs_file_get_block); + if (err < 0) + pr_err("newly allocated blocks reclaim not implemented yet\n"); + return err; +} #else static int simplefs_write_begin(struct file *file, struct address_space *mapping, @@ -135,13 +206,11 @@ static int simplefs_write_begin(struct file *file, unsigned int flags, struct page **pagep, void **fsdata) -#endif { struct simplefs_sb_info *sbi = SIMPLEFS_SB(file->f_inode->i_sb); int err; uint32_t nr_allocs = 0; - /* Check if the write can be completed (enough space?) */ if (pos + len > SIMPLEFS_MAX_FILESIZE) return -ENOSPC; @@ -153,26 +222,29 @@ static int simplefs_write_begin(struct file *file, if (nr_allocs > sbi->nr_free_blocks) return -ENOSPC; - /* prepare the write */ -#if SIMPLEFS_AT_LEAST(6, 12, 0) - err = block_write_begin(mapping, pos, len, foliop, simplefs_file_get_block); -#elif SIMPLEFS_AT_LEAST(5, 19, 0) - err = block_write_begin(mapping, pos, len, pagep, simplefs_file_get_block); -#else err = block_write_begin(mapping, pos, len, flags, pagep, simplefs_file_get_block); -#endif - /* if this failed, reclaim newly allocated blocks */ if (err < 0) pr_err("newly allocated blocks reclaim not implemented yet\n"); return err; } +#endif /* Called by the VFS after writing data from a write() syscall to the page * cache. This function updates inode metadata and truncates the file if * necessary. */ -#if SIMPLEFS_AT_LEAST(6, 12, 0) +#if SIMPLEFS_AT_LEAST(6, 15, 0) +static int simplefs_write_end(const struct kiocb *iocb, + struct address_space *mapping, + loff_t pos, + unsigned int len, + unsigned int copied, + struct folio *foliop, + void *fsdata) +{ + struct inode *inode = iocb->ki_filp->f_inode; +#elif SIMPLEFS_AT_LEAST(6, 12, 0) static int simplefs_write_end(struct file *file, struct address_space *mapping, loff_t pos, @@ -180,6 +252,8 @@ static int simplefs_write_end(struct file *file, unsigned int copied, struct folio *foliop, void *fsdata) +{ + struct inode *inode = file->f_inode; #else static int simplefs_write_end(struct file *file, struct address_space *mapping, @@ -188,9 +262,9 @@ static int simplefs_write_end(struct file *file, unsigned int copied, struct page *page, void *fsdata) -#endif { struct inode *inode = file->f_inode; +#endif struct simplefs_inode_info *ci = SIMPLEFS_INODE(inode); struct super_block *sb = inode->i_sb; #if SIMPLEFS_AT_LEAST(6, 6, 0) @@ -199,7 +273,10 @@ static int simplefs_write_end(struct file *file, uint32_t nr_blocks_old; /* Complete the write() */ -#if SIMPLEFS_AT_LEAST(6, 12, 0) +#if SIMPLEFS_AT_LEAST(6, 15, 0) + int ret = + generic_write_end(iocb, mapping, pos, len, copied, foliop, fsdata); +#elif SIMPLEFS_AT_LEAST(6, 12, 0) int ret = generic_write_end(file, mapping, pos, len, copied, foliop, fsdata); #else @@ -242,9 +319,15 @@ static int simplefs_write_end(struct file *file, /* Read ei_block to remove unused blocks */ bh_index = sb_bread(sb, ci->ei_block); if (!bh_index) { +#if SIMPLEFS_AT_LEAST(6, 15, 0) + pr_err("Failed to truncate '%s'. Lost %llu blocks\n", + iocb->ki_filp->f_path.dentry->d_name.name, + nr_blocks_old - inode->i_blocks); +#else pr_err("Failed to truncate '%s'. Lost %llu blocks\n", file->f_path.dentry->d_name.name, nr_blocks_old - inode->i_blocks); +#endif goto end; } index = (struct simplefs_file_ei_block *) bh_index->b_data; @@ -486,7 +569,9 @@ const struct address_space_operations simplefs_aops = { #else .readpage = simplefs_readpage, #endif +#if !SIMPLEFS_AT_LEAST(6, 15, 0) .writepage = simplefs_writepage, +#endif .write_begin = simplefs_write_begin, .write_end = simplefs_write_end, }; diff --git a/inode.c b/inode.c index 4ba8b73..634a935 100644 --- a/inode.c +++ b/inode.c @@ -147,8 +147,10 @@ static struct dentry *simplefs_lookup(struct inode *dir, /* Iterate blocks in extent */ for (bi = 0; bi < eblock->extents[ei].ee_len; bi++) { bh2 = sb_bread(sb, eblock->extents[ei].ee_start + bi); - if (!bh2) + if (!bh2) { + brelse(bh); return ERR_PTR(-EIO); + } dblock = (struct simplefs_dir_block *) bh2->b_data; @@ -375,7 +377,7 @@ static void simplefs_set_file_into_dir(struct simplefs_dir_block *dblock, uint32_t inode_no, const char *name) { - int fi; + int fi = 0; if (dblock->nr_files != 0 && dblock->files[0].inode != 0) { for (fi = 0; fi < SIMPLEFS_FILES_PER_BLOCK - 1; fi++) { if (dblock->files[fi].nr_blk != 1) @@ -383,14 +385,18 @@ static void simplefs_set_file_into_dir(struct simplefs_dir_block *dblock, } dblock->files[fi + 1].inode = inode_no; dblock->files[fi + 1].nr_blk = dblock->files[fi].nr_blk - 1; - strncpy(dblock->files[fi + 1].filename, name, SIMPLEFS_FILENAME_LEN); + strncpy(dblock->files[fi + 1].filename, name, + SIMPLEFS_FILENAME_LEN - 1); + dblock->files[fi + 1].filename[SIMPLEFS_FILENAME_LEN - 1] = '\0'; dblock->files[fi].nr_blk = 1; } else if (dblock->nr_files == 0) { - dblock->files[fi].inode = inode_no; - strncpy(dblock->files[fi].filename, name, SIMPLEFS_FILENAME_LEN); + dblock->files[0].inode = inode_no; + strncpy(dblock->files[0].filename, name, SIMPLEFS_FILENAME_LEN - 1); + dblock->files[0].filename[SIMPLEFS_FILENAME_LEN - 1] = '\0'; } else { dblock->files[0].inode = inode_no; - strncpy(dblock->files[fi].filename, name, SIMPLEFS_FILENAME_LEN); + strncpy(dblock->files[0].filename, name, SIMPLEFS_FILENAME_LEN - 1); + dblock->files[0].filename[SIMPLEFS_FILENAME_LEN - 1] = '\0'; } dblock->nr_files++; } @@ -475,6 +481,12 @@ static int simplefs_create(struct inode *dir, dir_nr_files = eblock->nr_files; avail = simplefs_get_available_ext_idx(&dir_nr_files, eblock); + /* Validate avail index is within bounds */ + if (avail >= SIMPLEFS_MAX_EXTENTS) { + ret = -EMLINK; + goto iput; + } + /* if there is not any empty space, alloc new one */ if (!dir_nr_files && !eblock->extents[avail].ee_start) { ret = simplefs_put_new_ext(sb, avail, eblock); @@ -924,7 +936,16 @@ static int simplefs_rename(struct inode *old_dir, return ret; } -#if SIMPLEFS_AT_LEAST(6, 3, 0) +#if SIMPLEFS_AT_LEAST(6, 15, 0) +static struct dentry *simplefs_mkdir(struct mnt_idmap *id, + struct inode *dir, + struct dentry *dentry, + umode_t mode) +{ + int ret = simplefs_create(id, dir, dentry, mode | S_IFDIR, 0); + return ret ? ERR_PTR(ret) : NULL; +} +#elif SIMPLEFS_AT_LEAST(6, 3, 0) static int simplefs_mkdir(struct mnt_idmap *id, struct inode *dir, struct dentry *dentry, @@ -1003,6 +1024,12 @@ static int simplefs_link(struct dentry *old_dentry, int dir_nr_files = eblock->nr_files; avail = simplefs_get_available_ext_idx(&dir_nr_files, eblock); + /* Validate avail index is within bounds */ + if (avail >= SIMPLEFS_MAX_EXTENTS) { + ret = -EMLINK; + goto end; + } + /* if there is not any empty space, alloc new one */ if (!dir_nr_files && !eblock->extents[avail].ee_start) { ret = simplefs_put_new_ext(sb, avail, eblock); @@ -1035,6 +1062,7 @@ static int simplefs_link(struct dentry *old_dentry, /* write the file info into simplefs_dir_block */ simplefs_set_file_into_dir(dblock, old_inode->i_ino, dentry->d_name.name); + eblock->extents[avail].nr_files++; eblock->nr_files++; mark_buffer_dirty(bh2); mark_buffer_dirty(bh); @@ -1086,31 +1114,41 @@ static int simplefs_symlink(struct inode *dir, uint32_t avail; /* Check if symlink content is not too long */ - if (l > sizeof(ci->i_data)) - return -ENAMETOOLONG; + if (l > sizeof(ci->i_data)) { + ret = -ENAMETOOLONG; + goto iput; + } /* fill directory data block */ bh = sb_bread(sb, ci_dir->ei_block); - if (!bh) - return -EIO; + if (!bh) { + ret = -EIO; + goto iput; + } eblock = (struct simplefs_file_ei_block *) bh->b_data; if (eblock->nr_files == SIMPLEFS_MAX_SUBFILES) { ret = -EMLINK; printk(KERN_INFO "directory is full"); - goto end; + goto iput; } int dir_nr_files = eblock->nr_files; avail = simplefs_get_available_ext_idx(&dir_nr_files, eblock); + /* Validate avail index is within bounds */ + if (avail >= SIMPLEFS_MAX_EXTENTS) { + ret = -EMLINK; + goto iput; + } + /* if there is not any empty space, alloc new one */ if (!dir_nr_files && !eblock->extents[avail].ee_start) { ret = simplefs_put_new_ext(sb, avail, eblock); switch (ret) { case -ENOSPC: ret = -ENOSPC; - goto end; + goto iput; case -EIO: ret = -EIO; goto put_block; @@ -1136,6 +1174,7 @@ static int simplefs_symlink(struct inode *dir, /* write the file info into simplefs_dir_block */ simplefs_set_file_into_dir(dblock, inode->i_ino, dentry->d_name.name); + eblock->extents[avail].nr_files++; eblock->nr_files++; mark_buffer_dirty(bh2); mark_buffer_dirty(bh); @@ -1155,8 +1194,10 @@ static int simplefs_symlink(struct inode *dir, eblock->extents[ei].ee_len); memset(&eblock->extents[ei], 0, sizeof(struct simplefs_extent)); } - -end: +iput: + put_blocks(SIMPLEFS_SB(sb), ci->ei_block, 1); + put_inode(SIMPLEFS_SB(sb), inode->i_ino); + iput(inode); brelse(bh); return ret; }