Skip to content

Commit d4666f5

Browse files
de-nordicnashif
authored andcommitted
fs: Fix fs_open resource leak when invoked on fs_file_t object in use
Fixes problem when fs_open invoked on fs_file_t object, which is already holding information on opened file, overwrites references to other memory objects within the fs_file_t object causing resource leak. If fs_open is invoked on already used fs_file_t object, it will return -EBUSY. Note: The change requires that all fs_file_t objects should be initialized to 0. Fixes: #29478 Signed-off-by: Dominik Ermel <[email protected]>
1 parent 2132923 commit d4666f5

File tree

2 files changed

+18
-3
lines changed

2 files changed

+18
-3
lines changed

subsys/fs/fs.c

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -143,25 +143,30 @@ int fs_open(struct fs_file_t *zfp, const char *file_name, fs_mode_t flags)
143143
return -EINVAL;
144144
}
145145

146+
if (zfp->mp != NULL) {
147+
return -EBUSY;
148+
}
149+
146150
rc = fs_get_mnt_point(&mp, file_name, NULL);
147151
if (rc < 0) {
148152
LOG_ERR("%s:mount point not found!!", __func__);
149153
return rc;
150154
}
151155

152-
zfp->mp = mp;
153156
if (((mp->flags & FS_MOUNT_FLAG_READ_ONLY) != 0) &&
154157
(flags & FS_O_CREATE || flags & FS_O_WRITE)) {
155158
return -EROFS;
156159
}
157160

158-
CHECKIF(zfp->mp->fs->open == NULL) {
161+
CHECKIF(mp->fs->open == NULL) {
159162
return -ENOTSUP;
160163
}
161164

162-
rc = zfp->mp->fs->open(zfp, file_name, flags);
165+
zfp->mp = mp;
166+
rc = mp->fs->open(zfp, file_name, flags);
163167
if (rc < 0) {
164168
LOG_ERR("file open error (%d)", rc);
169+
zfp->mp = NULL;
165170
return rc;
166171
}
167172

tests/subsys/fs/fs_api/src/test_fs_dir_file.c

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -436,6 +436,10 @@ void test_file_open(void)
436436
ret = fs_open(&filep, TEST_FILE, FS_O_READ);
437437
zassert_equal(ret, 0, "Fail to open file");
438438

439+
TC_PRINT("\nDouble-open\n");
440+
ret = fs_open(&filep, TEST_FILE, FS_O_READ);
441+
zassert_equal(ret, -EBUSY, "Expected -EBUSY, got %d", ret);
442+
439443
TC_PRINT("\nReopen the same file");
440444
ret = fs_open(&filep, TEST_FILE, FS_O_READ);
441445
zassert_not_equal(ret, 0, "Reopen an opend file");
@@ -827,6 +831,12 @@ void test_file_close(void)
827831
ret = fs_close(&filep);
828832
zassert_equal(ret, 0, "Fail to close file");
829833

834+
TC_PRINT("Reuse fs_file_t from closed file");
835+
ret = fs_open(&filep, TEST_FILE, FS_O_READ);
836+
zassert_equal(ret, 0, "Expected open to succeed, got %d", ret);
837+
ret = fs_close(&filep);
838+
zassert_equal(ret, 0, "Expected close to succeed, got %d", ret);
839+
830840
TC_PRINT("\nClose a closed file:\n");
831841
filep.mp = &test_fs_mnt_1;
832842
ret = fs_close(&filep);

0 commit comments

Comments
 (0)