Skip to content

Conversation

@jserv
Copy link
Collaborator

@jserv jserv commented Nov 24, 2025

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 *'

Summary by cubic

Fixes high-severity SimpleFS bugs that could leak buffers, corrupt directory entries, or mis-handle block allocation. Adds Linux 6.15+ compatibility for write paths and mkdir to keep builds green and runtime stable.

  • Bug Fixes

    • Allocation: on sb_bread failure, restore bitmap and free-block count, and return 0 (reserved) instead of -EIO.
    • simplefs_lookup: release buffer on error to avoid a leak.
    • Directory entries: initialize indices and null-terminate filenames after strncpy to prevent undefined behavior.
    • Extents: validate avail index in create/link/symlink and increment extent nr_files in link/symlink.
  • Compatibility

    • Update write_begin/write_end to 6.15+ kiocb signatures, adjust inode/file access, and keep older branches; add space checks before block_write_begin.
    • Drop writepage from aops on 6.15+ (deprecated).
    • simplefs_mkdir returns struct dentry* on 6.15+.

Written for commit fe5f56b. Summary will update automatically on new commits.

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 *'
@jserv jserv force-pushed the resolve-severity-issues branch 2 times, most recently from c69ad94 to aaf82c8 Compare November 24, 2025 09:29
Copy link

@cubic-dev-ai cubic-dev-ai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

1 issue found across 3 files

Prompt for AI agents (all 1 issues)

Understand the root cause of the following 1 issues and fix them.


<file name="inode.c">

<violation number="1" location="inode.c:1030">
Returning -EMLINK via the new `avail &gt;= SIMPLEFS_MAX_EXTENTS` branch leaks the inode allocated by `simplefs_new_inode()` because the `goto end` path never frees the inode or its block.</violation>
</file>

Reply to cubic to teach it or ask questions. Re-run a review with @cubic-dev-ai review this PR

/* Validate avail index is within bounds */
if (avail >= SIMPLEFS_MAX_EXTENTS) {
ret = -EMLINK;
goto end;
Copy link

@cubic-dev-ai cubic-dev-ai bot Nov 24, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Returning -EMLINK via the new avail >= SIMPLEFS_MAX_EXTENTS branch leaks the inode allocated by simplefs_new_inode() because the goto end path never frees the inode or its block.

Prompt for AI agents
Address the following comment on inode.c at line 1030:

<comment>Returning -EMLINK via the new `avail &gt;= SIMPLEFS_MAX_EXTENTS` branch leaks the inode allocated by `simplefs_new_inode()` because the `goto end` path never frees the inode or its block.</comment>

<file context>
@@ -1003,6 +1024,12 @@ static int simplefs_link(struct dentry *old_dentry,
+    /* Validate avail index is within bounds */
+    if (avail &gt;= SIMPLEFS_MAX_EXTENTS) {
+        ret = -EMLINK;
+        goto end;
+    }
+
</file context>
Fix with Cubic

@jserv jserv force-pushed the resolve-severity-issues branch from aaf82c8 to fe5f56b Compare November 24, 2025 09:54
@jserv jserv merged commit 17e6c30 into master Nov 24, 2025
4 checks passed
@jserv jserv deleted the resolve-severity-issues branch November 28, 2025 04:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants