On 7/2/2025 9:41 AM, Johannes Berg wrote: > On Wed, 2025-07-02 at 21:55 +0530, Sarika Sharma wrote: >> Currently, cfg80211_sinfo_release_content() frees dynamically >> allocated memory but does not reset the associated pointers. >> This results in double free issues in nl80211_dump_station(), >> where both link_sinfo and link_sinfo->pertid are released twice, >> once after the send_station() call and again in the error handling path. >> >> Hence, to fix accidental dereferencing of dangling pointers, explicitly >> set the freed pointers to NULL. >> > > Do we have to fix it this way? It feels like perhaps it should rather be > fixed by only having one call to cfg80211_sinfo_release_content() in > each path. Internally Sarika had a 2-patch series where the first patch attempts to make sure cfg80211_sinfo_release_content() is only called when needed. However that required either separating out the error cases into separate gotos or calling cfg80211_sinfo_release_content() at each error point, and I felt that unnecessarily made the code less readable & maintainable, especially given that the 2nd patch (which is the one she posted) addresses the underlying problem (and is similar to the approach Dan posted where he zeroes sinfo at the end of cfg80211_sinfo_release_content()). In my mind this is no different than the numerous places where we blindly call kfree() with a pointer that may or may not be NULL. But if you want the other patch as well, we can continue our internal review of it. /jeff
