Skip to content

Memory-safety fixes: uninitialized reads and a realloc leak - #613

Open
matejk wants to merge 2 commits into
LinearTapeFileSystem:mainfrom
matejk:fix/memory-safety
Open

Memory-safety fixes: uninitialized reads and a realloc leak#613
matejk wants to merge 2 commits into
LinearTapeFileSystem:mainfrom
matejk:fix/memory-safety

Conversation

@matejk

@matejk matejk commented Jun 12, 2026

Copy link
Copy Markdown
Contributor

Two commits:

  • Reads of uninitialized values on error paths (4 sites): tape_get_pews leaves *pews unset on -LTFS_UNSUPPORTED while the caller computes pews + 10; ltfs_profiler_set tested an unset ret when the volume had neither an iosched nor a device handle; _get_dump derived a transfer length from cap_buf without checking the READ BUFFER result; the ICU normalize callers compared the output pointer against the input before checking the return code, reading uninitialized memory and potentially leaking the input buffer.
  • Key-list leak in the simple KMI: realloc was assigned straight back to priv.dk_list, so a failure overwrote the only pointer to the existing buffer with NULL and leaked it; the original pointer is now kept and freed on the error path.

@vandelvan
vandelvan requested review from XV02, madjesc and syaoraang June 12, 2026 20:55
@matejk
matejk force-pushed the fix/memory-safety branch from 1c27543 to 418c898 Compare June 17, 2026 20:35

@XV02 XV02 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks for all your work, just some correctness comments :DD

Comment thread src/kmi/simple.c Outdated
Comment thread src/libltfs/ltfs.c Outdated
Comment thread src/libltfs/pathname.c Outdated
Comment thread src/libltfs/pathname.c Outdated
Comment thread src/libltfs/tape.c Outdated
Comment thread src/tape_drivers/linux/sg/sg_tape.c Outdated
@matejk
matejk force-pushed the fix/memory-safety branch from 418c898 to fcd7a4c Compare August 17, 2026 20:10
@matejk
matejk changed the base branch from main to release/v2.4.9.0 August 17, 2026 20:10
@matejk
matejk changed the base branch from release/v2.4.9.0 to main August 17, 2026 20:11
@matejk
matejk force-pushed the fix/memory-safety branch from fcd7a4c to a73c937 Compare August 17, 2026 20:13
matejk added 2 commits August 17, 2026 22:19
pews, ret in ltfs_profiler_set, cap_buf after a failed READ BUFFER, and the
ICU normalize output pointer compared before its return code was checked.
@matejk
matejk force-pushed the fix/memory-safety branch 2 times, most recently from 8040315 to 18cb1e6 Compare August 17, 2026 20:28
@matejk

matejk commented Aug 17, 2026

Copy link
Copy Markdown
Contributor Author

Thanks — addressed:

  • The comments on the initializations and NULL checks are removed.
  • simple.c: back to the single error path; the original pointer is kept in previous and freed there. free(NULL) is a no-op on the calloc branch, so no extra condition is needed.

make check 14/14.

@matejk
matejk requested a review from XV02 August 17, 2026 20:31

@XV02 XV02 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks for addressing all my comments, LGTM! :DD

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