Skip to content

Commit 9c202f7

Browse files
committed
posix: shm: fix fd leak on error paths and protect shm_list with mutex
In subsys/portability/posix/options/shm.c, shm_open() reserves a file descriptor using zvfs_reserve_fd() before checking whether the target shm object exists or is unlinked. On error conditions where the shared memory object does not exist (when O_CREAT is omitted) or where the object has already been unlinked, shm_open() returned -1 with errno set to ENOENT or EACCES without calling zvfs_free_fd(fd). This caused a permanent file descriptor leak in the global ZVFS descriptor table (CONFIG_ZVFS_OPEN_MAX). Additionally, the global doubly-linked list shm_list and object reference counters were manipulated without synchronization across concurrent shm_open(), shm_unlink(), and shm_close() calls. Fix both issues by: 1. Releasing reserved file descriptors via zvfs_free_fd(fd) on all error return paths in shm_open(). 2. Protecting shm_list traversal, insertion, removal, and reference counting with a dedicated mutex (shm_lock). 3. Adding a test in xsi_realtime verifying that repeated failed open attempts do not leak file descriptors. Signed-off-by: harshit kudhial <harshitkudhial@gmail.com>
1 parent e64f6be commit 9c202f7

2 files changed

Lines changed: 45 additions & 0 deletions

File tree

  • subsys/portability/posix/options
  • tests/subsys/portability/posix/xsi_realtime/src

subsys/portability/posix/options/shm.c

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@
2525
static const struct fd_op_vtable shm_vtable;
2626

2727
static sys_dlist_t shm_list = SYS_DLIST_STATIC_INIT(&shm_list);
28+
static K_MUTEX_DEFINE(shm_lock);
2829

2930
struct shm_obj {
3031
uint8_t *mem;
@@ -239,10 +240,12 @@ static int shm_close(void *obj)
239240
{
240241
struct shm_obj *shm = obj;
241242

243+
k_mutex_lock(&shm_lock, K_FOREVER);
242244
shm->refs -= (shm->refs > 0) ? 1 : 0;
243245
if (shm->unlinked && (shm->refs == 0)) {
244246
shm_obj_remove(shm);
245247
}
248+
k_mutex_unlock(&shm_lock);
246249

247250
return 0;
248251
}
@@ -336,15 +339,21 @@ int shm_open(const char *name, int oflag, mode_t mode)
336339
}
337340

338341
key = hash32(name, name_len);
342+
343+
k_mutex_lock(&shm_lock, K_FOREVER);
344+
339345
shm = shm_obj_find(key);
340346
if ((shm != NULL) && shm->unlinked) {
341347
/* we cannot open a shm object that has already been unlinked */
348+
k_mutex_unlock(&shm_lock);
349+
zvfs_free_fd(fd);
342350
errno = EACCES;
343351
return -1;
344352
}
345353

346354
if (creat) {
347355
if ((shm != NULL) && excl) {
356+
k_mutex_unlock(&shm_lock);
348357
zvfs_free_fd(fd);
349358
errno = EEXIST;
350359
return -1;
@@ -353,6 +362,7 @@ int shm_open(const char *name, int oflag, mode_t mode)
353362
if (shm == NULL) {
354363
shm = k_calloc(1, sizeof(*shm));
355364
if (shm == NULL) {
365+
k_mutex_unlock(&shm_lock);
356366
zvfs_free_fd(fd);
357367
errno = ENOSPC;
358368
return -1;
@@ -362,11 +372,15 @@ int shm_open(const char *name, int oflag, mode_t mode)
362372
shm_obj_add(shm);
363373
}
364374
} else if (shm == NULL) {
375+
k_mutex_unlock(&shm_lock);
376+
zvfs_free_fd(fd);
365377
errno = ENOENT;
366378
return -1;
367379
}
368380

369381
++shm->refs;
382+
k_mutex_unlock(&shm_lock);
383+
370384
zvfs_finalize_typed_fd(fd, shm, &shm_vtable, ZVFS_MODE_IFSHM);
371385

372386
return fd;
@@ -384,8 +398,12 @@ int shm_unlink(const char *name)
384398
}
385399

386400
key = hash32(name, name_len);
401+
402+
k_mutex_lock(&shm_lock, K_FOREVER);
403+
387404
shm = shm_obj_find(key);
388405
if ((shm == NULL) || shm->unlinked) {
406+
k_mutex_unlock(&shm_lock);
389407
errno = ENOENT;
390408
return -1;
391409
}
@@ -395,5 +413,7 @@ int shm_unlink(const char *name)
395413
shm_obj_remove(shm);
396414
}
397415

416+
k_mutex_unlock(&shm_lock);
417+
398418
return 0;
399419
}

tests/subsys/portability/posix/xsi_realtime/src/shm.c

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -200,3 +200,28 @@ ZTEST(xsi_realtime, test_shm_mmap)
200200

201201
zassert_ok(shm_unlink(VALID_SHM_PATH));
202202
}
203+
204+
ZTEST(xsi_realtime, test_shm_open_fd_leak)
205+
{
206+
int fd[N];
207+
208+
/* Repeatedly attempt to open nonexistent shm without O_CREAT */
209+
for (size_t i = 0; i < N * 2; ++i) {
210+
int bad_fd = shm_open("/nonexistent_shm", OPEN_FLAGS, VALID_MODE);
211+
212+
zassert_equal(bad_fd, -1, "shm_open should fail for nonexistent shm");
213+
zassert_equal(errno, ENOENT, "errno should be ENOENT");
214+
}
215+
216+
/* Verify all N file descriptor slots remain available */
217+
for (size_t i = 0; i < N; ++i) {
218+
fd[i] = shm_open(VALID_SHM_PATH, i == 0 ? CREATE_FLAGS : OPEN_FLAGS, VALID_MODE);
219+
zassert_true(fd[i] >= 0, "shm_open(%s) failed after failed opens: fd=%d, errno=%d",
220+
VALID_SHM_PATH, fd[i], errno);
221+
}
222+
223+
zassert_ok(shm_unlink(VALID_SHM_PATH));
224+
for (size_t i = N; i > 0; --i) {
225+
zassert_ok(close(fd[i - 1]));
226+
}
227+
}

0 commit comments

Comments
 (0)