View Issue Details
| ID | Project | Category | View Status | Date Submitted | Last Update |
|---|---|---|---|---|---|
| 0006186 | GNUnet | util library | public | 2020-04-17 10:32 | 2021-09-02 16:14 |
| Reporter | fefe | Assigned To | Christian Grothoff | Due Date | |
| Priority | normal | Severity | trivial | Reproducibility | N/A |
| Status | closed | Resolution | fixed | ||
| Product Version | 0.12.1 | ||||
| Target Version | 0.13.0 | Fixed in Version | 0.13.0 | ||
| Summary | 0006186: Why does GNUNET_free not set the pointer to NULL? | ||||
| Description | One of the more notorious sources of errors in C is that free() will leave the pointer around, even though it no longer points to anything. That enables security holes like double free or other types of memory corruption. GNUNET comes with a wrapper around free called GNUNET_free, and it is a macro, so it could set the pointer to NULL after the freeing is done. It does not. Why? | ||||
| Tags | No tags attached. | ||||
| Not sure how to do it that works. Suppose I use: #define GNUNET_free(ptr) do { \ GNUNET_xfree_ (ptr, __FILE__, __LINE__); \ ptr = NULL; \ } while (0) it immediately bombs on things like: gnunet-resolver.c: In function ‘main’: ../../src/include/gnunet_common.h:1294:9: error: lvalue required as left operand of assignment ptr = NULL; \ ^ gnunet-resolver.c:181:3: note: in expansion of macro ‘GNUNET_free’ GNUNET_free ((void *) argv); | |
| I'm happy to #define a GNUNET_freez() that additionally sets to NULL, but as the example above shows, that won't _always_ work. | |
| fefe: any better suggestions? | |
| Remove the void* cast maybe? It looks superfluous to me? Maybe you can trick something with gcc extensions here, but I think your freez suggestion looks good. However I'd do it the other way around. Make GNUNET_free set the pointer to zero, and add an exceptional version for the few cases where that won't work. That may however break other third party code using that gnunet macro. | |
| There are cases (rare, admittedly), where the pointer might have been 'const', and still must be free'd. Now, that itself may be ugly, but I'd still want to support it. I'll investigate just how badly it breaks existing code bases (those I know of) if we change the default of GNUNET_free to set to zero and have a GNUNET_free_nz for the 'exceptions'. Based that, it'll be easier to decide if it's acceptable to break compatibility like that. | |
| zero-ing out is now the default, GNUNET_nz remains (for const casting and stuff like GNUNET_free(fun()) which happens, but rarely. | |
| 0.13.0 released | |
| Fix committed to master branch. | |
| Date Modified | Username | Field | Change |
|---|---|---|---|
| 2020-04-17 10:32 | fefe | New Issue | |
| 2020-04-17 16:23 | Christian Grothoff | Note Added: 0015682 | |
| 2020-04-17 16:25 | Christian Grothoff | Note Added: 0015683 | |
| 2020-04-18 11:15 | Christian Grothoff | Status | new => feedback |
| 2020-04-20 21:50 | Christian Grothoff | Note Added: 0015712 | |
| 2020-04-20 21:57 | fefe | Note Added: 0015713 | |
| 2020-04-20 21:57 | fefe | Status | feedback => new |
| 2020-04-20 22:14 | Christian Grothoff | Note Added: 0015714 | |
| 2020-04-20 23:01 | Christian Grothoff | Assigned To | => Christian Grothoff |
| 2020-04-20 23:01 | Christian Grothoff | Status | new => resolved |
| 2020-04-20 23:01 | Christian Grothoff | Resolution | open => fixed |
| 2020-04-20 23:01 | Christian Grothoff | Fixed in Version | => 0.12.2 |
| 2020-04-20 23:01 | Christian Grothoff | Note Added: 0015715 | |
| 2020-04-20 23:01 | Christian Grothoff | Target Version | => 0.12.2 |
| 2020-04-23 08:45 | schanzen | Fixed in Version | 0.12.2 => 0.13.0 |
| 2020-04-23 08:47 | schanzen | Target Version | 0.12.2 => 0.13.0 |
| 2020-05-31 22:49 | Issue cloned: 0006313 | ||
| 2020-05-31 22:52 | Issue cloned: 0006345 | ||
| 2020-07-09 07:17 | schanzen | Note Added: 0016434 | |
| 2020-07-09 07:17 | schanzen | Status | resolved => closed |
| 2021-09-02 16:13 | Christian Grothoff | ||
| 2021-09-02 16:14 | Christian Grothoff | Note Added: 0018263 |