mailing list of musl libc
 help / color / mirror / code / Atom feed
From: "Uwe Kleine-König" <uwe+openwrt@kleine-koenig.org>
To: Rich Felker <dalias@libc.org>
Cc: musl@lists.openwall.com
Subject: Re: [musl] nslookup failures with coarse CLOCK_MONOTONIC
Date: Sat, 8 Oct 2022 02:37:30 +0200	[thread overview]
Message-ID: <c77d5fd0-63b6-1156-7c44-f5ac2b816839@kleine-koenig.org> (raw)
In-Reply-To: <20221007232540.GC29905@brightrain.aerifal.cx>

Hello Rich,

On 10/8/22 01:25, Rich Felker wrote:
> On Sat, Oct 08, 2022 at 01:04:25AM +0200, Uwe Kleine-König wrote:
>>
>> Musl does the following to create the 16 bit ID:
>>
>>           /* Make a reasonably unpredictable id */
>>           clock_gettime(CLOCK_REALTIME, &ts);
>>           id = ts.tv_nsec + ts.tv_nsec/65536UL & 0xffff;
>>           q[0] = id/256;
>>           q[1] = id;
>>
>> (from musl's src/network/res_mkquery.c) My hypothesis now is that
>> the monotonic clock has a resolution of 20 µs only. So if the two
>> res_mkquery() calls are called within the same 20 µs tick, the IDs
>> end up being identical. If they happen in two consecutive ticks, the
>> IDs have a delta of 20000 or 20001 which matches the four cases
>> observed above.
>>
>> To improve the situation I suggest something like:
>>
>> diff --git a/src/network/res_mkquery.c b/src/network/res_mkquery.c
>> index 614bf7864b48..78b3095fe959 100644
>> --- a/src/network/res_mkquery.c
>> +++ b/src/network/res_mkquery.c
>> @@ -11,6 +11,7 @@ int __res_mkquery(int op, const char *dname, int
>> class, int type,
>>          struct timespec ts;
>>          size_t l = strnlen(dname, 255);
>>          int n;
>> +       static unsigned int querycnt;
>>
>>          if (l && dname[l-1]=='.') l--;
>>          if (l && dname[l-1]=='.') return -1;
>> @@ -34,6 +35,8 @@ int __res_mkquery(int op, const char *dname, int
>> class, int type,
>>
>>          /* Make a reasonably unpredictable id */
>>          clock_gettime(CLOCK_REALTIME, &ts);
>> +       /* force a different ID if mkquery was called twice during
>> the same tick */
>> +       ts.tv_nsec += querycnt++;
>>          id = ts.tv_nsec + ts.tv_nsec/65536UL & 0xffff;
>>          q[0] = id/256;
>>          q[1] = id;
>>
>> Would that make sense?
> 
> This isn't acceptable as-is because it introduces a data race.

Huh. You mean if there is a race the pseudo random IDs are changed in a 
platform dependent way? Doesn't sound sooo bad to me, but probably I'm 
too naive here. Also res_mkquery is documented to be not thread-safe. I 
didn't check deeply, but I guess the counter should be moved to struct 
__res_state. (Assuming the manpage for res_mkquery also applies to musl.)

Despite your concerns, I updated the libc on my RE200 with the above 
patch now, and I cannot observe the failure any more.

Of course updating nslookup to use a better API is a nicer solution.

> Which implementation of nslookup is this? Busybox? It would probably
> be useful to hear thoughts on it from their side.

Jo already replied here, nothing to add from my side.

BTW, glibc also generates the IDs from the monotonic clock only. It 
makes a bit bigger effort to scramble the bits in there, but if the time 
doesn't advance, it also generates identical IDs.

Best regards and thanks for your quick response
Uwe

  parent reply	other threads:[~2022-10-08  1:24 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-10-07 23:04 Uwe Kleine-König
2022-10-07 23:25 ` Rich Felker
2022-10-07 23:53   ` Jo-Philipp Wich
2022-10-08  0:05     ` Rich Felker
2022-10-08  0:37   ` Uwe Kleine-König [this message]
2022-10-08  7:07     ` Markus Wichmann

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=c77d5fd0-63b6-1156-7c44-f5ac2b816839@kleine-koenig.org \
    --to=uwe+openwrt@kleine-koenig.org \
    --cc=dalias@libc.org \
    --cc=musl@lists.openwall.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://git.vuxu.org/mirror/musl/

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).