fs/mtd: clean up FTL allocations on initialization failure by shin4141 · Pull Request #7549 · Samsung/TizenRT

shin4141 · 2026-09-29T00:27:06Z

fs/mtd: clean up FTL allocations on initialization failure

Problem

ftl_initialize() and ftl_nand_initialize() allocate child buffers before registering the block driver. Several later failure paths free only the device structure. register_blockdriver() assigns the private device pointer to an inode only after inode_reserve() succeeds, so the initializer still owns those buffers when registration fails.

Changes

Release the FTL erase buffer and read/write buffers after partial buffer initialization or failed block-driver registration.
Release the NAND FTL block map and optional erase buffer on their later failure paths; clear both pointers after release on registration failure.
Zero-initialize the FTL device's inactive buffer/work state and return early if device allocation fails. Initialize the active buffer semaphores only after all enabled buffer allocations succeed. If read-ahead allocation fails, rwb_initialize() releases the previously allocated write buffer; the FTL caller then releases only its own erase buffer and device.
Check read/write callbacks during initialization only for active buffers. A disabled direction can have a zero buffer length and no callback; no additional fallback callback assertions are needed for the FTL call paths.

On successful registration, the initializer leaves the device and its buffers in place. Existing return codes and read/write logic are unchanged.

Validation

An isolated macOS host harness compiled the actual three changed C source files with stubbed kernel interfaces, AddressSanitizer, and UndefinedBehaviorSanitizer. It injects allocation, geometry, and registration failures and counts live allocations, semaphore initialization/destruction, and double frees.

Both FTL buffers enabled: original source 9/14 cases passed; final patch 14/14 passed. The five original failures were the two FTL buffer allocation paths, FTL registration, NAND erase-buffer allocation, and NAND registration.
Final patch: writable/no-buffer 12/12; read-only/no-buffer 10/10; FTL write-only with both global buffer features enabled 13/13; FTL read-ahead-only with both global features enabled 13/13; read-only FTL read-ahead with both global features enabled 11/11; FTL write-only with global write buffering alone 13/13.
All tested failure paths ended with zero tracked allocations and semaphores, zero double frees, and zero invalid semaphore destruction. Successful registration retained the device resources and its geometry callback worked. No sanitizer report was produced. git diff --check passed.

The global read-ahead-only configuration remains outside the tested matrix. Pristine master and the initially submitted patch produced the same five host compilation errors in existing write-buffer timeout helpers that reference write-buffer fields while global write buffering is disabled. This pre-existing build problem is outside this fix; the revised patch was not separately compiled in that configuration.

This validation does not include a TizenRT firmware build, QEMU/board execution, real flash operations, the real kernel workqueue, sem_init() failure injection, or field frequency/impact measurements. The existing le_tc tests do not provide deterministic failure injection for this private initializer, so the host harness is not presented as a native TizenRT regression test.

AI assistance: OpenAI Codex helped analyze the code, prepare the local patch, and build the isolated harness. The test conditions and unverified areas are stated above for maintainer review.

Taejun-Kwon · 2026-10-02T00:07:29Z


 #ifdef CONFIG_DRVR_WRITEBUFFER
-	DEBUGASSERT(rwb->wrflush != NULL);
+	DEBUGASSERT(rwb->wrmaxblocks == 0 || rwb->wrflush != NULL);


rwb->wrmaxblocks == 0 will not come forever, because rwb->wrmaxblocks is same as dev->blkper.
blkper is calculated based on size of eraseblock & read/write block.
Each of flash driver based on MTD using constant value, I mean defined value.
If you really want to check wrmaxblocks , then it need to be checked after block driver check geometry(for example, https://github.com/Samsung/TizenRT/blob/master/os/fs/driver/mtd/ftl.c#L526), not here.

Thanks. I removed the wrmaxblocks == 0 || check and placed the callback assertion inside the active write-buffer branch. The FTL read-ahead-only configuration can leave the write direction inactive.

Taejun-Kwon · 2026-10-02T00:09:17Z

-	DEBUGASSERT(rwb->rhreload != NULL);
+	DEBUGASSERT(rwb->rhmaxblocks == 0 || rwb->rhreload != NULL);
 	rwb->rhbuffer = NULL;
+	if (rwb->rhmaxblocks > 0) {


half is right, but half is wrong. please see line 655. both of sem can be initialize at line 686

Thanks. The semaphores for enabled buffers are now initialized only after all enabled buffer allocations succeed. If read-ahead allocation fails, rwb_initialize() frees the earlier write buffer before returning.

Taejun-Kwon · 2026-10-02T00:11:50Z

 		 * the user buffer.
 		 */

+		DEBUGASSERT(rwb->rhreload != NULL);


It's not necessary, rwb->rhreload already checked in rwb_initialize.
if rwb_initialize failed because rwb->rhreload is null then block driver will not be registered.
In this case pseudo file system will not include path of ftl driver, so file operation(open, read, write) never be called.

Removed the extra direct read/write fallback callback assertions. Active callbacks are checked during rwb_initialize() before block-driver registration.

Taejun-Kwon · 2026-10-02T00:37:48Z

 		ret = rwb_initialize(&dev->rwb);
 		if (ret < 0) {
 			dbg("ERROR: rwb_initialize failed: %d\n", ret);
+			rwb_uninitialize(&dev->rwb);


we clenad resource if rwb_initialize failed, so I think we should not call rwb_uninitialize here.

Agreed. FTL no longer calls rwb_uninitialize() after rwb_initialize() fails. The helper rolls back its partial buffer allocation, while FTL frees its erase buffer and device. Registration failure still uses rwb_uninitialize().

Taejun-Kwon · 2026-10-02T00:40:42Z

 			dbg("ERROR: rwb_initialize failed: %d\n", ret);
+			rwb_uninitialize(&dev->rwb);
+#ifdef CONFIG_FS_WRITABLE
+			kmm_free(dev->eblock);


can you please consider something more?
I think early-return is good option for this code (line 515);

Applied the early return when the FTL device allocation fails. It returns -ENOMEM as before, and the remaining initialization keeps the same ownership and cleanup paths.

Taejun-Kwon · 2026-10-02T00:43:17Z

 		if (ret < 0) {
 			dbg("ERROR: register_blockdriver failed: %d\n", -ret);
+#ifdef CONFIG_FS_WRITABLE
+			kmm_free(dev->eblock);


please add below more.

dev->eblock = NULL; dev->block_map = NULL;

Added dev->eblock = NULL and dev->block_map = NULL immediately after their respective frees on registration failure.

Taejun-Kwon

thanks for your effort.

Taejun-Kwon · 2026-10-02T03:42:31Z


 #ifdef CONFIG_DRVR_WRITEBUFFER
 	if (rwb->wrmaxblocks > 0) {
+		DEBUGASSERT(rwb->wrflush != NULL);


then line 623 can be moved to between line 629 & 630 to reduce code line. Same opinion for rhbuffer.

Moved the wrbuffer and rhbuffer NULL assignments before their buffer checks and removed the duplicate assignments. Thanks for the suggestion.

shin4141 added 2 commits September 29, 2026 08:46

fs/mtd: release FTL buffers on initialization failure

5729b97

drivers/rwbuffer: validate callbacks only for active buffers

689edd4

Taejun-Kwon reviewed Oct 2, 2026

View reviewed changes

fs/mtd: address FTL initialization review feedback

2bcdc57

Taejun-Kwon reviewed Oct 2, 2026

View reviewed changes

drivers/rwbuffer: simplify buffer pointer initialization

fa73665

Taejun-Kwon approved these changes Oct 15, 2026

View reviewed changes

Taejun-Kwon merged commit b26151d into Samsung:master Oct 15, 2026

Conversation

shin4141 commented Sep 29, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

fs/mtd: clean up FTL allocations on initialization failure

Problem

Changes

Validation

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Taejun-Kwon left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

shin4141 commented Sep 29, 2026 •

edited

Loading