free()

[Axel Freyn]

Dear Mohsen,

again - that won't work correctly:
> I changed given func to the following func:
> //////////////////////////////////////////////////////////////////
> void safe_free(void * p){
>         memset(&p, 0, sizeof(p));
>         free(p);
>         p = 0;
> }//end of safe_free func
> ///////////////////////////////////////////////////////////

a) as "p" is a pointer, sizeof(p) will be the size of a pointer on your
   machine. This will be probably 4 or 8 bytes (for 32bit or 64bit
   machines) - indepenently of the type of p! So your memset won't
   overwrite all the memory where p points to, but only the first 4 or 8
   bytes. 

b) in your call to memset, you use "&p" instead of p. So instead of
   passing the memory address where p points to, you pass the address
   where this memory address is stored (that is, the variable "p"). This
   address is the overwritten by 0 ==> you set p to zero, BEFORE you
   call "free(p)". Again, this results in a memory leak: you won't free
   the memory.

c) your call "memset(&p, 0, sizeof(p));" does exactly the same as 
   "p = 0;": you set the pointer to zero.

d) as you shifted everything into a separate function, the "p" in this
   function will be a copy of the original pointer. Setting the copy to
   zero does NOT change the original pointer. Imagine (analogous to your
   first example:

   p1 = (linked_list*) malloc(sizeof(linked_list));
   safe_free(p1);
   printf ("%s",p1->srcip);

   now, p1 will continue to point to the memory allocated by malloc; the
   printf will work.


In fact, I don't understand at the moment, what you try to achieve
exactly?

 - Destroy the information that was stored in "p", so that nobody can
   read it from memory later (e.g. some attacker who reads out the
   complete memory): Therefor, you have to zero-out the memory as
   Steffen suggested. But to do that, you MUST know the size of the
   memory where a given pointer points to. When you use normal "malloc",
   you can't obtain this information later: if you only have a pointer
   "void *p", it is impossible to know how many memory is allocated at
   this address (well, of course it's possible, but you need support
   from the Kernel or your standard C library...). If your code knows
   it, you can do it: e.g. for your example: when you KNOW that p
   points to a single element of type "struct linked_liste",
   "memset(p, 0 , sizeof(struct linked_list));" will work. If you don't
   know the size or you want to write a generic function, you NEED a
   special library for malloc / free, which takes care to zero-out the
   memory an which remembers internally the size of memory pointed to by
   each pointer.

 - Make it impossible for your program, to use "p" to access this
   memory: "free(p);p=0;" is sufficient. However, that does NOT zero-out
   the data, so that everybody how can read the memory of your machine
   will be able to obtain the informations stored in p later (until that
   memory is overwritten by another program, which will happen sooner or
   later).

 - Just free the memory, so that it can be reused later by other
   programs or by your program to store another variable: "free(p);" is
   sufficient. This makes ist not impossible to use "p" to access the
   memory - however, when you do it, the behaviour is undefined: it may
   work - or it may result in a segmentation fault (when the kernel
   reserved that memory already for another program running on your
   machine), or it might result in really arbitrary behaviour of your
   program (if that memory is already used by another part of your
   program...).

Axel
> 
> On Sun, 2009-09-27 at 14:13 +0200, Axel Freyn wrote:
> > Dear Mohsen,
> > 
> > you are speaking about different things;-)
> > Steffen proposed, to zero out the Memory - in order to guarantee, that
> > nobody will be able to access the data where "p" pointed to
> > That could be done by
> > memset(p, 0, sizeof(struct linked_list));
> > what you did (p=0;) is CHANGING the Pointer: you set the address, where
> > p points to, to zero - and AFTERWARDS you free this address: you try to
> > free the address "0". The result of your code is a memory leak: the
> > memory you allocated for p before will never be freed.
> > The "safest" way would be:
> > 
> > memset(p, 0, sizeof(struct linked_liste));
> > free(p);
> > p=0;
> > 
> > so: 
> >  - first, set the memory where p points to, to zero
> >  - second, free this memory
> >  - third, set the pointer to zero.
> > 
> > HTH,
> > 
> > Axel
> > 
> > On Sun, Sep 27, 2009 at 03:32:01AM +0330, Mohsen Pahlevanzadeh wrote:
> > > Dear Steffen & all,
> > > According to your description, i wrote following func & it work well:
> > > ///////////////////////////////////////////////////////
> > > void safe_free(void * p){
> > >         p = 0;
> > >         free(p);
> > > }//end of safe_free func
> > > ///////////////////////////////////////////////////////
> > > Thank you.
> > > 
> > > On Sun, 2009-09-27 at 11:37 +0200, Steffen Wendzel wrote:
> > > > You have to zero the memory it before, like I described here:
> > > > 
> > > > http://www.wendzel.de/dr.org/libcmle/examples/mem.html
> > > > 
> > > > Steffen
> > > > 
> > > > On Sun, 27 Sep 2009 00:18:00 +0330
> > > > Mohsen Pahlevanzadeh <mohsen@pahlevanzadeh.org> wrote:
> > > > 
> > > > > Dear all,
> > > > > We are working on C code (not ++),So we must use free instead delete.
> > > > > I have following code:
> > > > > /////////////////////////////////
> > > > >         struct linked_list *p;
> > > > > 	p->src="10.0.0.1";
> > > > >         free(p);
> > > > >         printf ("%s",p->srcip);
> > > > > /////////I see in my output 10.0.0.1 
> > > > > My question: i drop p pinter, but see it's value, how i kill p with its
> > > > > value?
> > > > > 
> > > > > Yours,
> > > > > Mohsen
> > > > > 
> > > > 
> > > > 
> > > 
>