free a pointer and have it point to NULL

S

Stefan Ram

G G said:
memory allocated on the heap once freed should the pointer be assigned NULL?

There is no general answer possible.

It is not necessary, but might be used to follow
project-specific guidelines for memory management
or as »defensive programming«.

To get a more specific answer, post an SSCCE.
 
J

James Kuyper

memory allocated on the heap once freed should the pointer be assigned NULL?

Only if there's a danger of the pointer being used again after being
free()'d. Even then, it's only useful if other code, elsewhere, checks
if(p) before trying to dereference p. Finally, if there are multiple
saved pointers, all pointing into the same block of allocated memory,
setting one of those pointers to be null won't help avoid problems with
any of the other pointers. They must all be protected by if(p).

I often use the following approach, which removes all need for nulling
out the free()'d pointer:

{
int *p = malloc(num_elements*sizeof *p);
if(p==NULL)
{
// Error handling
}
else
{
// Code which uses *p
free(p);
}
// Key point: *p is not used anywhere in this section
}

Since the lifetime of 'p' ends immediately after free(p), there's no
need to write "p = NULL;". This approach cannot be used if the malloc()
and free() must occur in different functions, or at very different
levels within the same function.
 
K

Keith Thompson

G G said:
memory allocated on the heap once freed should the pointer be assigned NULL?

Maybe.

After free(ptr), the value of ptr is *indeterminate*, meaning that any
attempt to refer to its value has undefined behavior. In practice, just
reading the pointer probably isn't going to do anything harmful. But
having such a pointer value sitting around creates a potential source of
bugs, and setting it to NULL can remove *some* such bugs, or at least
change them into different bugs that might be easier to detect.

On the other hand, if the pointer object is just about to go out of
scope and therefore cease to exist, there's no point in setting it to
NULL. Example:

{
unsigned char *buffer = malloc(some_size);
/* error checking omitted */
/* code that uses the pointer omitted */
free(buffer);
}

Setting buffer to NULL after calling free() would be pointless, because
there's no way to refer to its value accidentally.
 
J

Jens Thoms Toerring

G G said:
memory allocated on the heap once freed should the pointer be assigned NULL?

It can't hurt since any attempt to use the now invalid pointer
again would lead to an immediate crash of the program. But then
it depends a bit on circumstances: if the free() is at the end
of a function and there's no chance that the pointer is acci-
dentally used again, then there's not much good it will do.
And, of course, each free() will have to be followed by
another line of code, which can become tedious and make
things a bit harder to read - unless you use a macro like

#define safe_free( addr ) \
do { \
free( addr ); \
addr = NULL; \
} while ( 1 == 0 )

instead of free() everywhere. I've done that in several cases
where I felt the need to be paranoid;-)

Regards, Jens
 
K

Keith Thompson

It can't hurt since any attempt to use the now invalid pointer
again would lead to an immediate crash of the program. But then
it depends a bit on circumstances: if the free() is at the end
of a function and there's no chance that the pointer is acci-
dentally used again, then there's not much good it will do.
And, of course, each free() will have to be followed by
another line of code, which can become tedious and make
things a bit harder to read - unless you use a macro like

#define safe_free( addr ) \
do { \
free( addr ); \
addr = NULL; \
} while ( 1 == 0 )

instead of free() everywhere. I've done that in several cases
where I felt the need to be paranoid;-)

"do { ... } while (0)" is the usual idiom for writing a macro that can
be used in a context requiring a statement; the "1 == 0" just seems
(mildly) obfuscated.

But when possible, it's usually better to write a macro that can be used
wherever an expression can appear. In this case:

#define safe_free(addr) ( free(addr), (addr) = NULL )
 
K

Kaz Kylheku

memory allocated on the heap once freed should the pointer be assigned NULL?

Once you free a pointer, all variables and other storage locations which previously
held that pointer's value now hold an indeterminate value which must not be used.

So, you don't lose any valuable information by overwriting some of the copies
of that value, or all of them, with null. On the contrary, you are scrubbing
away dangerous information.

If you do this in an existing program that appears to work, it may now crash with
a null pointer dereference. This indicates that it was relying on the
indeterminate value: it has a use-after-free bug which was revealed by changing
the pointer to null.

In some programs, you must set certain pointers to null after freeing, because
parts of the program specificaly check for the value and rely on it as an
indication "the usual object is not there". Should you design everything with
such checks? The answer is no: there are situations in which an object is just
expected to be there for the entire lifetime of another object, and so a null
check doesn't make sense.

So:

- Overwriting stale pointers with nulls is a good idea for turning
unpredictable behavior (use of garbage poitners) into null pointer
dereferences (a condition that, on many platforms, produces some predictable
exception which can be used to pinpoint the location and debug the program).
But watch out: retroactively doing this in a program can suddenly reveal
bugs, and you are on the hook for them because it looks like your change
"broke" the program. So you should do this when you're ready to take
responsibility to follow up and fix the null pointer dereferences that
may be revealed.

- Whether you use the null pointer as a special "not there value" and actually
check for it in key places in the code is debatable. It hinges on whether
it is reasonable for an object which is reachable through the pointer to
to sometimes not be available. If this is NOT reasonable (i.e. the pointer
should always be non-null and valid, except perhaps in very eearly
initialization stages of the code, or late in the finalization), tne you will
only confuse yourself by adding checks for null. When you add these checks,
then someone who reads the code (like yourself, six weeks later) begins to
suspect that it is expected behavior and fine for the pointer to sometimes be
null. And that makes the code look more complicated than it is. People then
waste time adding more such code, thinking it is necessary, testing it, and
looking for bugs which cannot exist.
 
J

Joe Pfeiffer

It can't hurt since any attempt to use the now invalid pointer
again would lead to an immediate crash of the program. But then
it depends a bit on circumstances: if the free() is at the end
of a function and there's no chance that the pointer is acci-
dentally used again, then there's not much good it will do.
And, of course, each free() will have to be followed by
another line of code, which can become tedious and make
things a bit harder to read - unless you use a macro like

#define safe_free( addr ) \
do { \
free( addr ); \
addr = NULL; \
} while ( 1 == 0 )

instead of free() everywhere. I've done that in several cases
where I felt the need to be paranoid;-)

Unfortunately, this doesn't help with situations like

a = malloc(size);
b = a;
safe_free(a);

so it's always struck me as a giving a false sense of security.
 
K

Kaz Kylheku

Unfortunately, this doesn't help with situations like

a = malloc(size);
b = a;
safe_free(a);

so it's always struck me as a giving a false sense of security.

it helps in situations like

safe_free (proxy->obj); /* proxy is the only gateway to obj */

In situations where you you deal with the shared reference problem, perhaps
with reference counting, it also helps. Even if proxy is not the only way to
reach obj, it makes sense to do:

drop_ref (proxy->obj); /* drop_ref is a macro which also sets obj to null */

proxy may well have had the last reference to obj. We have no right to access
obj through proxy, even if obj still exists. Since we have no right to access
obj through proxy, it's best to null out the pointer, so that any attempt to do
so is flagged as a null pointer dereference.

In other words, the management of multiple copies is a separate, problem; it
doesn't invalidate the setting of just one copy to null, which can still a
valuable approach when we have somehow addressed the management of sharing.
 
J

Joe Pfeiffer

Kaz Kylheku said:
it helps in situations like

safe_free (proxy->obj); /* proxy is the only gateway to obj */

In situations where you you deal with the shared reference problem, perhaps
with reference counting, it also helps. Even if proxy is not the only way to
reach obj, it makes sense to do:

drop_ref (proxy->obj); /* drop_ref is a macro which also sets obj to null */

proxy may well have had the last reference to obj. We have no right to access
obj through proxy, even if obj still exists. Since we have no right to access
obj through proxy, it's best to null out the pointer, so that any attempt to do
so is flagged as a null pointer dereference.

In other words, the management of multiple copies is a separate, problem; it
doesn't invalidate the setting of just one copy to null, which can still a
valuable approach when we have somehow addressed the management of sharing.

I don't think it's entirely separate -- when I see someone suggest
zeroing freed pointers in isolation, my impression is that they're
expecting that, in and of itself, to be helpful in spotting use of freed
memory. Without integrating into a shared reference management system,
I don't see it as useful.
 
A

Andrew Cooper

It can't hurt since any attempt to use the now invalid pointer
again would lead to an immediate crash of the program.

This is simply not true, relying on the assumption will end in pain (or
at least subtle bugs). I come from an x86 OS development background
where the problem is particularly prevalent.

The set of systems where the value 0 is actually an invalid pointer is
increasingly small. All x86 systems as well as most ARM and MIPS
systems can be up so the value 0 is a genuine pointer[1]. Under the
POSIX standard by which UNIX as well as Linux and the BSDs abide,
functions such as mmap()[2] can be used to put valid memory under a
pointer whose value is 0.

From an OS point of view, accidentally dereferencing a NULL pointer is
particularity bad as, while it will tend to result in a crash, malicious
userspace can use the bug to its advantage.


I wish to add that I am not advocating avoiding the use of NULL. I
would certainly encourage people to explicitly invalidate their pointers
if the pointers are not immediately going out of scope[3]. However,
don't fall into the trap of assuming that use of a NULL pointer will
result in a program crash.

It is unfortunate that implementations of 64bit x86 code don't use
0x8000000000000000 (or something similar) as the machine representation
of a NULL pointer. For the foreseeable future, 62/64ths of the virtual
address space will unconditionally generate a processor exception
irrespective of other state. I suspect this was down to inertia more
than the fact that most people can't write portable C.

~Andrew



[1] In 32bit mode without paging, the only way to get at the system IVT
is to create a pointer to the value 0 and use it.

[2] OpenBSD takes the precaution of actively preventing mapping memory
regions to 0 or 0x00007ffffffff000. This in turned out to be a
VeryGoodIdea from a security point of view as it protected them against
CVE-2012-0217.

[3] Any static analyser worth its salt will pick up accidental reuse of
a NULL pointer. And you are programming with the aid of a static
analyser aren't you...
 
K

Kaz Kylheku

This is simply not true, relying on the assumption will end in pain (or
at least subtle bugs). I come from an x86 OS development background
where the problem is particularly prevalent.

You make some good points, but:

* The null pointer is what the C language gives us, however it is defined
by the implementation. It is C's special pointer value which can be
accessed, but not displaced or dereferenced (with any defined
results, anyway), and which doesn't point to any object.

* If you use some techniques like mmap to put an object at the address
that is implied by the null pointer, you're sticking a fork in the
toaster.

* Compilers and debugging tools can check for null pointers without
assistance from memory management hardware. I seem to remember that some
popular compilers for the MS-DOS environment had a code generation option for
this. If my life depended on it, I could patch gcc into doing it.
Also, I think that the Valgrind debugger on Linux will still report null
pointer dereferences even if the program uses mmap to make the zero page
valid. So really, it's not a smart thing to do. In Linux kernel mode, null
pointers are trapped also; the kernel painstakingly arranges this for itself.

* MMU null pointer checks are not perfect, indeed; that is valid point.
For instance if there is only a 4096 byte unmapped page at the
zero address, x is null, but the expression x->memb is such that the offset
of memb is greater than 4096, then the expression can evaluate
without triggering an exception. Just because a measure doesn't always
work doesn't mean it's not worthwhile. Soap doesn't remove all stains;
we still wash mostly with soap.

* It is valid for a compiler to define 0x8000000000000000 as the null pointer.
Unfortunately, it will break lots of nonportable code which assumes that
pointers located in memory which comes from calloc, or which was memset to
zero, are implicitly initialized to null (and can be tested as being equal to
null). We can jump up and down and fume about such code being nonportable;
doing so won't make it go away.
 
G

glen herrmannsfeldt

(snip, someone wrote)
This is simply not true, relying on the assumption will end in pain (or
at least subtle bugs). I come from an x86 OS development background
where the problem is particularly prevalent.

It is probably better than leaving the previous value, though,
as it is likely that the previous data is still there for a little
while.
The set of systems where the value 0 is actually an invalid pointer is
increasingly small. All x86 systems as well as most ARM and MIPS
systems can be up so the value 0 is a genuine pointer[1]. Under the
POSIX standard by which UNIX as well as Linux and the BSDs abide,
functions such as mmap()[2] can be used to put valid memory under a
pointer whose value is 0.

On protected mode x86, from the 80286 on, segment selector zero is
a special null selector. The value in a segment register has to be
either a valid selector or zero. But that only matters when using
more than one segment, which is rare to nonexistent.

(Last I knew, the Watcom compilers could generate large mode 32 bit
code, and possibly OS/2 could run it.)
From an OS point of view, accidentally dereferencing a NULL pointer is
particularity bad as, while it will tend to result in a crash, malicious
userspace can use the bug to its advantage.
I wish to add that I am not advocating avoiding the use of NULL. I
would certainly encourage people to explicitly invalidate their pointers
if the pointers are not immediately going out of scope[3]. However,
don't fall into the trap of assuming that use of a NULL pointer will
result in a program crash.

I once had to debug a program, written by someone else, that would
get into an infinite loop. It turned out that it was trying to free
a linked list that either wasn't allocated or had been previously
deallocated. It used the usual recursive delete algorithm, but the
actual list, after over 100 links, had a loop in it. Somehow
hundreds of pointers were actually pointing to other pointers,
instead of some invalid address.

(snip)

-- glen
 
Ö

Öö Tiib

"do { ... } while (0)" is the usual idiom for writing a macro that can
be used in a context requiring a statement; the "1 == 0" just seems
(mildly) obfuscated.

I have seen even 'while (__LINE__ == -1)' there to silence a warning
of particular compiler that sometimes complained about condition
being constant expression (like on case of '0' or '1 == 0' but not on
case of '__LINE__ == -1').
 
J

James Kuyper

On 04/24/2014 03:38 AM, �� Tiib wrote:
....
I have seen even 'while (__LINE__ == -1)' there to silence a warning
of particular compiler that sometimes complained about condition
being constant expression (like on case of '0' or '1 == 0' but not on
case of '__LINE__ == -1').

__LINE__ is replaced by the actual line number in translation phase 4;
from that point on, if the line number happens to be 54, that construct
is indistinguishable from 54 == -1. I wonder how that compiler made its
decision not to warn about that construct, while warning about 1==0?
 
J

Joe Pfeiffer

Just came across this interesting story about openssl and the new
libressl project:

http://arstechnica.com/information-...eyond-repair-claims-creator-of-libressl-fork/

Following the links from the story sends one to a changelog with
possibly the densest collection of coding WTFs I've ever seen:

http://freshbsd.org/search?project=openbsd&q=file.name:libssl
(and I'd argue that these are enough plain-bad-C WTFs as opposed to
system-specific WTFs that it's on topic here).

There's even a "greatest hits" blog:

http://opensslrampage.org/post/82973125757/strncpy-dest-src-strlen-src-is-safe-right

I note (and here's why I put it in this thread) that the libressl team
disagrees with me on the utility of nulling freed pointers.
 
I

Ian Collins

The Sun Studio compiler, for one, has a horrible function call diagnostic
pass that runs before macros are expanded. It's really annoying.

For example, I have code like:

static struct fifo *fifo_init(struct fifo *, void *, size_t);

#define fifo_init3(...) (fifo_init)(__VA_ARGS__)
#define fifo_init1(fifo) fifo_init3((fifo), (void *)0, 0U)
#define fifo_init(...) XPASTE(fifo_init, NARG(__VA_ARGS__))(__VA_ARGS__)

If you use the 1-argument form, the code uses dynamic memory. Otherwise it
uses the fixed-sized buffer provided. When using the 1-argument form, Sun
Studio complains about passing the wrong number of arguments to the
fifo_init routine.
Eh?

cat m.c
#include <stdlib.h>

struct fifo;

struct fifo *fifo_init(struct fifo *, void *, size_t);

#define fifo_init3(...) (fifo_init)(__VA_ARGS__)
#define fifo_init1(fifo) fifo_init3((fifo), (void *)0, 0U)
#define fifo_init(...) XPASTE(fifo_init, NARG(__VA_ARGS__))(__VA_ARGS__)

void f( struct fifo* ff )
{
fifo_init3( ff, NULL, 0 );
fifo_init1( ff );
}
 
I

Ian Collins

william@solaris:/tmp$ cat m.c
#include <stdlib.h>

#define NARG_(_8, _7, _6, _5, _4, _3, _2, _1, N, ...) N
#define NARG(...) NARG_(__VA_ARGS__, 8, 7, 6, 5, 4, 3, 2, 1)

#define PASTE(a, b) a ## b
#define XPASTE(a, b) PASTE(a, b)

struct fifo;

struct fifo *fifo_init(struct fifo *, void *, size_t);

#define fifo_init3(...) (fifo_init)(__VA_ARGS__)
#define fifo_init1(fifo) fifo_init3((fifo), (void *)0, 0U)
#define fifo_init(...) XPASTE(fifo_init, NARG(__VA_ARGS__))(__VA_ARGS__)

void f( struct fifo* ff ) {
fifo_init(ff);
}

william@solaris:/tmp$ c99 m.c -c
"m.c", line 18: warning: argument mismatch

The warning makes sense, see:

# gcc -std=c99 -Wall -pedantic ~/temp/m.c -c
/home/ian/temp/m.c: In function 'f':
/home/ian/temp/m.c:18:21: warning: ISO C99 requires rest arguments to be
used

# gcc -std=c99 -Wall -pedantic ~/temp/m.c -E
....
void f( struct fifo* ff ) {
/home/ian/temp/m.c:18:21: warning: ISO C99 requires rest arguments to be
used
(fifo_init)((ff), (void *)0, 0U);
}

# c99 ~/temp/m.c -E
void f( struct fifo* ff ) {
(fifo_init)((ff), (void *)0, 0U);
 
Ö

Öö Tiib

On 04/24/2014 03:38 AM, �� Tiib wrote:
...

__LINE__ is replaced by the actual line number in translation phase 4;
from that point on, if the line number happens to be 54, that construct
is indistinguishable from 54 == -1. I wonder how that compiler made its
decision not to warn about that construct, while warning about 1==0?

I do not know but I can try to hypothesise. Compiler vendors have
always put (increasing over time) effort into static analysis and
diagnostics of code. Sometimes warnings are amazingly clever,
sometimes just annoying. Some are byproducts of optimisation.
When compiler can optimise it out then why it is written? May be
it deserves warning? That can be case with constant condition.

It is possible that phase 4 "sets a flag" of the value 54 being "variable"
enough and so not deserving warnings when compiler optimises the
whole branch, switch or loop out that uses it as condition.
 

Ask a Question

Want to reply to this thread or ask your own question?

You'll need to choose a username for the site, which only take a couple of moments. After that, you can post your question and our members will help you out.

Ask a Question

Members online

No members online now.

Forum statistics

Threads
473,743
Messages
2,569,478
Members
44,899
Latest member
RodneyMcAu

Latest Threads

Top