[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date: Thu, 11 Apr 2013 20:03:15 -0400
From: Zvi Gilboa <zg7s@...rvices.virginia.edu>
To: <musl@...ts.openwall.com>
Subject: Re: crypt_data: structure size in crypt.c vs. crypt.h
On 04/11/2013 06:59 PM, Rich Felker wrote:
> On Thu, Apr 11, 2013 at 06:42:26PM -0400, Zvi Gilboa wrote:
>> Greetings,
>>
>> Looking at crypt.c and crypt.h, I was wondering whether having
>> different-size /crypt_data/ structures was intentional, and if so,
>> for what reason. At the moment, the declarations are (and see also
>> the attached code):
> Yes and no. struct crypt_data becomes part of the ABI of the program
> that calls crypt_r, so it needs to be "large enough for any future
> password hash". The number 256 was chosen as a nice round value. On
> the other hand, the static buffer used by crypt() only needs to be
> "large enough for any currently-supported password hash", which is
> presently 128.
>
>> /<crypt.h>/
>> struct crypt_data {
>> int initialized;
>> char *__buf[256]*;
>> }; /* 260 bytes when sizeof(int)==4 */
>>
>> /<crypt.c>/
>> char *__crypt_r(const char *, const char *, *struct crypt_data **);
>>
>> char *crypt(const char *key, const char *salt)
>> {
>> static char *buf[128]*;
>> return __crypt_r(key, salt, *(struct crypt_data *)buf*);
>> /* when sizeof(int)==4, this leaves __buf with 124 bytes. */
> No, the size is the full 128. There really is no such member as
> "initialized" as far as musl's __crypt_r is concerned; the whole
> object is used simply as a char[] buffer. The only reason the
> "initialized" member exists is to meet the API requirements of the
> crypt_r function -- per the API, applications are supposed to either
> set crypt_data.initialized=0, or zero-initialize the whole object,
> before calling crypt_r. If a member named "initialized" did not exist,
> such programs would fail to compile. However, musl itself has no use
> for any initialized member, since it only uses crypt_data as an output
> buffer, not a working buffer for crypto state.
>
>> On that note: since the /initialized /member is (currently) never
>> referenced by name, adding a comment about that to the code might
>> help readers who are yet to be initiated:)
> Where do you think would be best to add it? In __crypt_r? The
> "natural" place would be the headers, but the current policy is to
> avoid comments in the headers, especially ones which would be
> implementation-specific rather than documenting the API.
Thanks for the explanation! Since crypt.c is likely to be visited by
anyone going through the crypt-related code, I'd suggest placing the
comment there (proposed comment attached).
>
> Rich
View attachment "crypt_data_comment.patch" of type "text/x-patch" (919 bytes)
Powered by blists - more mailing lists
Confused about mailing lists and their use?
Read about mailing lists on Wikipedia
and check out these
guidelines on proper formatting of your messages.
