View Issue Details

IDProjectCategoryView StatusLast Update
0006186GNUnetutil librarypublic2021-09-02 18:14
Reporterfefe Assigned ToChristian Grothoff  
PrioritynormalSeveritytrivialReproducibilityN/A
Status closedResolutionfixed 
Product Version0.12.1 
Target Version0.13.0Fixed in Version0.13.0 
Summary0006186: Why does GNUNET_free not set the pointer to NULL?
DescriptionOne 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?
TagsNo tags attached.

Activities

Christian Grothoff

2020-04-17 18:23

manager   ~0015682

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);

Christian Grothoff

2020-04-17 18:25

manager   ~0015683

I'm happy to #define a GNUNET_freez() that additionally sets to NULL, but as the example above shows, that won't _always_ work.

Christian Grothoff

2020-04-20 23:50

manager   ~0015712

fefe: any better suggestions?

fefe

2020-04-20 23:57

reporter   ~0015713

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.

Christian Grothoff

2020-04-21 00:14

manager   ~0015714

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.

Christian Grothoff

2020-04-21 01:01

manager   ~0015715

zero-ing out is now the default, GNUNET_nz remains (for const casting and stuff like GNUNET_free(fun()) which happens, but rarely.

schanzen

2020-07-09 09:17

administrator   ~0016434

0.13.0 released

Christian Grothoff

2021-09-02 18:14

manager   ~0018263

Fix committed to master branch.

Related Changesets

exchange: master 8148c1e8

2020-04-21 02:55

Christian Grothoff


Details Diff
fix 0006186 Affected Issues
0006186
mod - src/exchange/taler-exchange-aggregator.c Diff File
mod - src/exchange/taler-exchange-closer.c Diff File
mod - src/exchange/taler-exchange-transfer.c Diff File
mod - src/exchange/taler-exchange-wirewatch.c Diff File
mod - src/json/json_wire.c Diff File

Issue History

Date Modified Username Field Change
2020-04-17 12:32 fefe New Issue
2020-04-17 18:23 Christian Grothoff Note Added: 0015682
2020-04-17 18:25 Christian Grothoff Note Added: 0015683
2020-04-18 13:15 Christian Grothoff Status new => feedback
2020-04-20 23:50 Christian Grothoff Note Added: 0015712
2020-04-20 23:57 fefe Note Added: 0015713
2020-04-20 23:57 fefe Status feedback => new
2020-04-21 00:14 Christian Grothoff Note Added: 0015714
2020-04-21 01:01 Christian Grothoff Assigned To => Christian Grothoff
2020-04-21 01:01 Christian Grothoff Status new => resolved
2020-04-21 01:01 Christian Grothoff Resolution open => fixed
2020-04-21 01:01 Christian Grothoff Fixed in Version => 0.12.2
2020-04-21 01:01 Christian Grothoff Note Added: 0015715
2020-04-21 01:01 Christian Grothoff Target Version => 0.12.2
2020-04-23 10:45 schanzen Fixed in Version 0.12.2 => 0.13.0
2020-04-23 10:47 schanzen Target Version 0.12.2 => 0.13.0
2020-06-01 00:49 Adminknox Issue cloned: 0006313
2020-06-01 00:52 Adminknox Issue cloned: 0006345
2020-07-09 09:17 schanzen Note Added: 0016434
2020-07-09 09:17 schanzen Status resolved => closed
2021-09-02 18:13 Christian Grothoff Changeset attached => Taler-exchange master 8148c1e8
2021-09-02 18:14 Christian Grothoff Note Added: 0018263