OpenSSH 9.2 released

Posted Feb 3, 2023 17:39 UTC (Fri) by mss (subscriber, #138799)
In reply to: OpenSSH 9.2 released by ibukanov
Parent article: OpenSSH 9.2 released

free() and similar destructor-type functions in the middle of the function with no comments and no assignments of the pointer to NULL after that.

+1 for that, there's no reason not to set the object pointer to NULL after freeing the pointed-to object.
This turns a possible re-use into a NULL pointer dereference, which is almost always "just" a DoS risk.

For example, Glib has a nice g_clear_pointer () macro for doing exactly that, which makes it easy to audit the code that it does this clearing everywhere - just by searching for freestanding free () calls.

However, this does not replace clear object ownership rules and annotations.

For function-local objects or for holding a temporary object reference in a code block RAII-like self-freeing constructs as Glib's g_autoptr(TypeName) are very useful.
These especially make error cleanups much simpler (and these paths are often not well tested and bit-rot quickly).

AFAIK, Linux kernel would benefit from these constructs, too.

OpenSSH 9.2 released

Posted Feb 3, 2023 18:11 UTC (Fri) by ibukanov (subscriber, #3942) [Link] (2 responses)

Personally I do not like auto ptr and similar macros for C. I rather prefer explicit free at the end of a function with a static checker to make sure that it is always called. At least it makes porting code to weird embedded C compiler much less problematic.

As for rarely used cleanup paths a rather effective strategy IMO is to ban use of early return in C and always use goto instead to jump to the common cleanup code shared between success and error paths.

OpenSSH 9.2 released

Posted Feb 12, 2023 12:07 UTC (Sun) by nix (subscriber, #2304) [Link] (1 responses)

Ban use of early return, thus making early error checks much less readable, discouraging them, for the sake of edge cases like portability to weird embedded C compilers?

Aren't you optimizing for entirely the wrong thing here?

OpenSSH 9.2 released

Posted Feb 16, 2023 17:18 UTC (Thu) by mrugiero (guest, #153040) [Link]

From the mention to `goto` I infer OP means not using the `return` keyword directly but jumping to a label at the end of the function that does cleanup and then returns, rather than not exiting the function early on errors. It's a common pattern, I've seen it called "goto exit" and "goto cleanup".

OpenSSH 9.2 released

Posted Feb 7, 2023 9:58 UTC (Tue) by paulj (subscriber, #341) [Link]

For this reason, I like to have modules in C code of mine that allocate objects, have a foo_free(foo **obj) function. The foo module's free function can then NULL the pointer in the caller.

Far from perfect, the caller or other code may have stashed other pointers, but at least it ensures 'ownership' at the call site is cleanly transferred and destroyed.

More complete solutions would involve providing functions to handle transfer of assignment, rather than using C =, and/or weak pointer/reference abstraction. Whether that's worth it depends on the complexity of the lifetime management of whatever objects you're dealing with. (And many will say "Why not Rust these days instead?").