diff mbox series

[V2] resolv: use arc4random_uniform in the generation of random ids

Message ID 20220722213501.32506-1-crrodriguez@opensuse.org
State Changes Requested, archived
Headers
Series [V2] resolv: use arc4random_uniform in the generation of random ids |

Checks

Context Check Description
dj/TryBot-apply_patch success Patch applied to master at the time it was sent
dj/TryBot-32bit fail Patch caused testsuite regressions

Commit Message

Cristian Rodríguez July 22, 2022, 9:35 p.m. UTC 
  In the context of DNS it is important to have highest possible
quality ids.

v2: fix off by one calculation of upper limit, suggested by Florian.

Signed-off-by: Cristian Rodríguez <crrodriguez@opensuse.org>
---
 resolv/res_mkquery.c  | 3 +--
 resolv/res_randomid.c | 4 ++--
 2 files changed, 3 insertions(+), 4 deletions(-)

Comments

Carlos O'Donell July 24, 2022, 2:11 p.m. UTC | #1 
On Fri, Jul 22, 2022 at 5:35 PM Cristian Rodríguez via Libc-alpha
<libc-alpha@sourceware.org> wrote:
>
> In the context of DNS it is important to have highest possible
> quality ids.
>
> v2: fix off by one calculation of upper limit, suggested by Florian.
>
> Signed-off-by: Cristian Rodríguez <crrodriguez@opensuse.org>

This fails 32-bit pre-commit CI:
https://patchwork.sourceware.org/project/glibc/patch/20220722213501.32506-1-crrodriguez@opensuse.org/

Please review the CI results.

The link namespace results look like a real problem, we need an internal alias.

The mtrace leaks need review too.

> ---
>  resolv/res_mkquery.c  | 3 +--
>  resolv/res_randomid.c | 4 ++--
>  2 files changed, 3 insertions(+), 4 deletions(-)
>
> diff --git a/resolv/res_mkquery.c b/resolv/res_mkquery.c
> index 5bc5b41531..066ee2c97e 100644
> --- a/resolv/res_mkquery.c
> +++ b/resolv/res_mkquery.c
> @@ -93,7 +93,6 @@
>  #include <string.h>
>  #include <sys/time.h>
>  #include <shlib-compat.h>
> -#include <random-bits.h>
>
>  int
>  __res_context_mkquery (struct resolv_context *ctx, int op, const char *dname,
> @@ -116,7 +115,7 @@ __res_context_mkquery (struct resolv_context *ctx, int op, const char *dname,
>    /* We randomize the IDs every time.  The old code just incremented
>       by one after the initial randomization which still predictable if
>       the application does multiple requests.  */
> -  hp->id = random_bits ();
> +  hp->id = res_randomid ();
>    hp->opcode = op;
>    if (ctx->resp->options & RES_TRUSTAD)
>      hp->ad = 1;
> diff --git a/resolv/res_randomid.c b/resolv/res_randomid.c
> index fb1fa17539..b6211686c0 100644
> --- a/resolv/res_randomid.c
> +++ b/resolv/res_randomid.c
> @@ -84,10 +84,10 @@
>
>  #include <resolv.h>
>  #include <unistd.h>
> -#include <random-bits.h>
> +#include <stdlib.h>
>
>  unsigned int
>  res_randomid (void) {
> -  return 0xffff & random_bits ();
> +  return arc4random_uniform (UINT16_MAX + 1);
>  }
>  libc_hidden_def (__res_randomid)
> --
> 2.37.1
>
Cristian Rodríguez July 26, 2022, 11:39 p.m. UTC | #2 
On Sun, Jul 24, 2022 at 10:12 AM Carlos O'Donell
<carlos@systemhalted.org> wrote:
>
> On Fri, Jul 22, 2022 at 5:35 PM Cristian Rodríguez via Libc-alpha
> <libc-alpha@sourceware.org> wrote:
> >
> > In the context of DNS it is important to have highest possible
> > quality ids.
> >
> > v2: fix off by one calculation of upper limit, suggested by Florian.
> >
> > Signed-off-by: Cristian Rodríguez <crrodriguez@opensuse.org>
>
> This fails 32-bit pre-commit CI:
> https://patchwork.sourceware.org/project/glibc/patch/20220722213501.32506-1-crrodriguez@opensuse.org/
>
> Please review the CI results.
>
> The link namespace results look like a real problem, we need an internal alias.
>
> The mtrace leaks need review too.


Until the dust settles on the arc4random patches I won't be pushing
for these changes again
Cheers..
diff mbox series

Patch

diff --git a/resolv/res_mkquery.c b/resolv/res_mkquery.c
index 5bc5b41531..066ee2c97e 100644
--- a/resolv/res_mkquery.c
+++ b/resolv/res_mkquery.c
@@ -93,7 +93,6 @@ 
 #include <string.h>
 #include <sys/time.h>
 #include <shlib-compat.h>
-#include <random-bits.h>
 
 int
 __res_context_mkquery (struct resolv_context *ctx, int op, const char *dname,
@@ -116,7 +115,7 @@  __res_context_mkquery (struct resolv_context *ctx, int op, const char *dname,
   /* We randomize the IDs every time.  The old code just incremented
      by one after the initial randomization which still predictable if
      the application does multiple requests.  */
-  hp->id = random_bits ();
+  hp->id = res_randomid ();
   hp->opcode = op;
   if (ctx->resp->options & RES_TRUSTAD)
     hp->ad = 1;
diff --git a/resolv/res_randomid.c b/resolv/res_randomid.c
index fb1fa17539..b6211686c0 100644
--- a/resolv/res_randomid.c
+++ b/resolv/res_randomid.c
@@ -84,10 +84,10 @@ 
 
 #include <resolv.h>
 #include <unistd.h>
-#include <random-bits.h>
+#include <stdlib.h>
 
 unsigned int
 res_randomid (void) {
-  return 0xffff & random_bits ();
+  return arc4random_uniform (UINT16_MAX + 1);
 }
 libc_hidden_def (__res_randomid)