fix: guard dlt_log_handle close+reset under dlt_mutex (CWE-362, MISRA R1.3)#831
Open
aki1770-del wants to merge 1 commit intoCOVESA:masterfrom
Open
fix: guard dlt_log_handle close+reset under dlt_mutex (CWE-362, MISRA R1.3)#831aki1770-del wants to merge 1 commit intoCOVESA:masterfrom
aki1770-del wants to merge 1 commit intoCOVESA:masterfrom
Conversation
… R1.3) This fixes a data race on dlt_user.dlt_log_handle in dlt_user_log_send_log_v2() (the DLTv2 protocol send path). The close+reset sequence (close(dlt_user.dlt_log_handle); dlt_log_handle = -1;) in the DLT_RETURN_PIPE_ERROR case runs without holding dlt_mutex, while concurrent threads may be reading or writing dlt_log_handle via DLT_LOG_X() macros. Safety classification: - CWE-362: Race Condition (TOCTOU on dlt_log_handle check + close) - CWE-416: fd-reuse hazard (closed fd may be reallocated to unrelated resource) - MISRA C:2012 Rule 1.3: Undefined behavior (C11 §5.1.2.4 data race on non-atomic) - AUTOSAR SWS_DLT_01000: freedom from interference between concurrent DLT users The fix wraps the close+reset block under dlt_mutex_lock()/dlt_mutex_unlock(). dlt_mutex is a POSIX recursive mutex (PTHREAD_MUTEX_RECURSIVE) — no deadlock risk. The v1 send path (dlt_user_log_send_log) already acquires dlt_mutex at function entry; this patch closes the equivalent gap in the v2 path. Also removes an orphaned dlt_mutex_unlock() in dlt_free() that was left without a matching lock, introduced alongside the existing code. Note: dlt_user_init was made thread-safe in a prior patch (see ReleaseNotes). This PR closes the corresponding gap in the DLTv2 send path. Fixes: COVESA#253
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #253.
Problem
dlt_user.dlt_log_handleis closed and reset without holdingdlt_mutexin theDLT_RETURN_PIPE_ERRORcase ofdlt_user_log_send_log_v2()(src/lib/dlt_user.c):Concurrent threads calling
DLT_LOG_X()macros may readdlt_log_handlebetween theclose()and the= -1assignment (TOCTOU window), or write to a file descriptor that has already been closed and reallocated to an unrelated resource (fd-reuse hazard).Safety classification:
dlt_log_handlecheck + close)Fix
Wrap the
close+resetblock underdlt_mutex_lock()/dlt_mutex_unlock():dlt_mutexis aPTHREAD_MUTEX_RECURSIVEmutex — no deadlock risk from nested acquisition. The v1 send path (dlt_user_log_send_log()) already acquiresdlt_mutexat function entry; this patch closes the equivalent gap introduced in the v2 path.Secondary fix
Removes an orphaned
dlt_mutex_unlock()indlt_free()that was left without a matchingdlt_mutex_lock(), immediately after thedlt_receiver_free()block. This unbalanced unlock would corrupt the mutex state on any platform that detects unlock-without-lock.Continuity
dlt_user_initwas made thread-safe in a prior patch (seeReleaseNotes.md: "dlt_user: Make dlt_init thread safe"). This PR closes the corresponding gap in the deinitialization and DLTv2 send paths.ReleaseNotes.md("DLT MISRA conform changes"). This patch closes the most significant remaining MISRA-relevant gap indlt_user.c(Rule 1.3 — undefined behavior via data race).Change scope
src/lib/dlt_user.conly. +2 lines, −3 lines. No new dependencies. No new files. No behavior change on the non-error path.