casaroli commented on code in PR #17847:
URL: https://github.com/apache/nuttx/pull/17847#discussion_r4434996325
##########
mm/iob/iob_free.c:
##########
@@ -133,6 +133,13 @@ FAR struct iob_s *iob_free(FAR struct iob_s *iob)
kmm_free(iob);
}
+#ifdef CONFIG_IOB_OWNER_TRACKING
+ /* Clear owner metadata when returning to free list */
Review Comment:
With CONFIG_IOB_ALLOC this writes io_owner_* after iob->io_free(iob) or
kmm_free(iob) freed the IOB: a write to freed heap memory.
Fix: clear the owner fields before the IOB is freed.
##########
mm/iob/iob_free.c:
##########
@@ -196,6 +203,14 @@ FAR struct iob_s *iob_free(FAR struct iob_s *iob)
}
#endif
+#ifdef CONFIG_IOB_OWNER_TRACKING
+ /* Clear owner metadata when returning to free list */
Review Comment:
Here the IOB is already on the free or committed list, the lock is released
and nxsem_post() may have woken a waiter. If that task or an interrupt
allocates the IOB first, this clears its new owner, and /proc/iobinfo shows an
IOB in use as "Free".
Fix: clear the owner fields under g_iob_lock, before the IOB goes on the
list.
##########
include/nuttx/mm/iob.h:
##########
@@ -126,6 +126,12 @@ struct iob_s
#endif
unsigned int io_pktlen; /* Total length of the packet */
+ #ifdef CONFIG_IOB_OWNER_TRACKING
Review Comment:
` #ifdef CONFIG_IOB_OWNER_TRACKING` starts with a space, and
iob_get_iob_by_index() accepts index == CONFIG_IOB_NBUFFERS (`index >
CONFIG_IOB_NBUFFERS`), one past the last IOB.
Fix: remove the space and use `index >= CONFIG_IOB_NBUFFERS`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
