[prev in list] [next in list] [prev in thread] [next in thread] 

List:       krbdev
Subject:    Re: krb5_free_data
From:       "Markus Moeller" <huaraz () moeller ! plus ! com>
Date:       2014-06-22 14:33:21
Message-ID: lo6pfj$f0c$1 () ger ! gmane ! org
[Download RAW message or body]

Thank you. I will need to review a lot of code then :-(
Markus

"Greg Hudson"  wrote in message news:53A6E33E.2020702@mit.edu... 

On 06/22/2014 08:04 AM, Markus Moeller wrote:
> I see.  So if k5_pac_locate_buffer returns with an error I may get this 
> problem ?

Your code begins with:

>> ad_data = (krb5_data *)xmalloc(sizeof(krb5_data));

After this line, ad_data->data and ad_data->length are uninitialized.
If you were to immediately call krb5_free_data(context, ad_data) without
doing anything else, you would often see a crash because ad_data->data
is heap garbage.

If you use calloc instead of malloc, or call "memset(ad_data, 0,
sizeof(krb5_data))" after allocating, then you avoid this state.
(Strictly speaking, the C standard says that zeroed memory and null
pointers aren't necessarily the same thing, but lots of C code ignores
this and assumes that they are.  See http://c-faq.com/null/ for more on
that.)

Your code goes on to call:

>> #define KERB_LOGON_INFO 1
>>     ret = krb5_pac_get_buffer(context, pac, KERB_LOGON_INFO, ad_data);

If krb5_pac_get_buffer succeeds, ad_data->data and ad_data->length will
be filled in with the results, and you can safely free ad_data.

If it fails, it currently doesn't touch ad_data at all, leaving it
uninitialized and therefore unsafe to free.  Tom refers to this as
"probably a bug," but it's not uncommon for older libkrb5 APIs.  To be
safe, do not assume that libkrb5 APIs will initialize their output
parameters when they fail.
_______________________________________________
krbdev mailing list             krbdev@mit.edu
https://mailman.mit.edu/mailman/listinfo/krbdev

_______________________________________________
krbdev mailing list             krbdev@mit.edu
https://mailman.mit.edu/mailman/listinfo/krbdev
[prev in list] [next in list] [prev in thread] [next in thread]