[4/5] resolv: Enhance __resolv_conf_load to capture file change data
diff mbox series

Message ID 39fc4b71f02884315d1dacd5a5a0f1ee45009f12.1579631655.git.fweimer@redhat.com
State New
Headers show
Series
  • Race condition in /etc/resolv.conf reloading (bug 25420)
Related show

Commit Message

Florian Weimer Jan. 21, 2020, 6:42 p.m. UTC
The data is captured after reading the file.  This allows callers
to check the change data against an earlier measurement.
---
 resolv/res_init.c    | 14 +++++++++++---
 resolv/resolv_conf.c |  2 +-
 resolv/resolv_conf.h | 10 +++++++---
 3 files changed, 19 insertions(+), 7 deletions(-)

Comments

Adhemerval Zanella Feb. 13, 2020, 9:33 p.m. UTC | #1
On 21/01/2020 15:42, Florian Weimer wrote:
> The data is captured after reading the file.  This allows callers
> to check the change data against an earlier measurement.

LGTM, thanks.

Reviewed-by: Adhemerval Zanella <adhemerval.zanella@linaro.org>

> ---
>  resolv/res_init.c    | 14 +++++++++++---
>  resolv/resolv_conf.c |  2 +-
>  resolv/resolv_conf.h | 10 +++++++---
>  3 files changed, 19 insertions(+), 7 deletions(-)
> 
> diff --git a/resolv/res_init.c b/resolv/res_init.c
> index 09345718cd..98d84f264d 100644
> --- a/resolv/res_init.c
> +++ b/resolv/res_init.c
> @@ -103,6 +103,7 @@
>  #include <inet/net-internal.h>
>  #include <errno.h>
>  #include <resolv_conf.h>
> +#include <file_change_detection.h>
>  
>  static uint32_t net_mask (struct in_addr);
>  

Ok.

> @@ -549,7 +550,8 @@ res_vinit_1 (FILE *fp, struct resolv_conf_parser *parser)
>  }
>  
>  struct resolv_conf *
> -__resolv_conf_load (struct __res_state *preinit)
> +__resolv_conf_load (struct __res_state *preinit,
> +                    struct file_change_detection *change)
>  {
>    /* Ensure that /etc/hosts.conf has been loaded (once).  */
>    _res_hconf_init ();

Ok.

> @@ -577,7 +579,13 @@ __resolv_conf_load (struct __res_state *preinit)
>    resolv_conf_parser_init (&parser, preinit);
>  
>    struct resolv_conf *conf = NULL;
> -  if (res_vinit_1 (fp, &parser))
> +  bool ok = res_vinit_1 (fp, &parser);
> +  if (ok && change != NULL)
> +    /* Update the file change information if the configuration was
> +       loaded successfully.  */
> +    ok = file_change_detection_for_fp (change, fp);
> +
> +  if (ok)
>      {
>        parser.template.nameserver_list
>          = nameserver_list_begin (&parser.nameserver_list);

Ok.

> @@ -615,7 +623,7 @@ __res_vinit (res_state statp, int preinit)
>    if (preinit && has_preinit_values (statp))
>      /* For the preinit case, we cannot use the cached configuration
>         because some settings could be different.  */
> -    conf = __resolv_conf_load (statp);
> +    conf = __resolv_conf_load (statp, NULL);
>    else
>      conf = __resolv_conf_get_current ();
>    if (conf == NULL)

Ok.

> diff --git a/resolv/resolv_conf.c b/resolv/resolv_conf.c
> index d954ba9a5a..bdd2ebb909 100644
> --- a/resolv/resolv_conf.c
> +++ b/resolv/resolv_conf.c
> @@ -136,7 +136,7 @@ __resolv_conf_get_current (void)
>      {
>        /* Parse configuration while holding the lock.  This avoids
>           duplicate work.  */
> -      conf = __resolv_conf_load (NULL);
> +      conf = __resolv_conf_load (NULL, NULL);
>        if (conf != NULL)
>          {
>            if (global_copy->conf_current != NULL)

Ok.

> diff --git a/resolv/resolv_conf.h b/resolv/resolv_conf.h
> index 01cbff9111..101e14bfe5 100644
> --- a/resolv/resolv_conf.h
> +++ b/resolv/resolv_conf.h
> @@ -63,12 +63,16 @@ struct resolv_conf
>     and the struct resolv_context facility.  */
>  
>  struct __res_state;
> +struct file_change_detection;
>  
>  /* Read /etc/resolv.conf and return a configuration object, or NULL if
>     /etc/resolv.conf cannot be read due to memory allocation errors.
> -   If PREINIT is not NULL, some configuration values are taken from the
> -   struct __res_state object.  */
> -struct resolv_conf *__resolv_conf_load (struct __res_state *preinit)
> +   If PREINIT is not NULL, some configuration values are taken from
> +   the struct __res_state object.  If CHANGE is not null, file change
> +   detection data is written to *CHANGE, based on the state of the
> +   file after reading it.  */
> +struct resolv_conf *__resolv_conf_load (struct __res_state *preinit,
> +                                        struct file_change_detection *change)
>    attribute_hidden __attribute__ ((warn_unused_result));
>  
>  /* Return a configuration object for the current /etc/resolv.conf
> 

Ok.

Patch
diff mbox series

diff --git a/resolv/res_init.c b/resolv/res_init.c
index 09345718cd..98d84f264d 100644
--- a/resolv/res_init.c
+++ b/resolv/res_init.c
@@ -103,6 +103,7 @@ 
 #include <inet/net-internal.h>
 #include <errno.h>
 #include <resolv_conf.h>
+#include <file_change_detection.h>
 
 static uint32_t net_mask (struct in_addr);
 
@@ -549,7 +550,8 @@  res_vinit_1 (FILE *fp, struct resolv_conf_parser *parser)
 }
 
 struct resolv_conf *
-__resolv_conf_load (struct __res_state *preinit)
+__resolv_conf_load (struct __res_state *preinit,
+                    struct file_change_detection *change)
 {
   /* Ensure that /etc/hosts.conf has been loaded (once).  */
   _res_hconf_init ();
@@ -577,7 +579,13 @@  __resolv_conf_load (struct __res_state *preinit)
   resolv_conf_parser_init (&parser, preinit);
 
   struct resolv_conf *conf = NULL;
-  if (res_vinit_1 (fp, &parser))
+  bool ok = res_vinit_1 (fp, &parser);
+  if (ok && change != NULL)
+    /* Update the file change information if the configuration was
+       loaded successfully.  */
+    ok = file_change_detection_for_fp (change, fp);
+
+  if (ok)
     {
       parser.template.nameserver_list
         = nameserver_list_begin (&parser.nameserver_list);
@@ -615,7 +623,7 @@  __res_vinit (res_state statp, int preinit)
   if (preinit && has_preinit_values (statp))
     /* For the preinit case, we cannot use the cached configuration
        because some settings could be different.  */
-    conf = __resolv_conf_load (statp);
+    conf = __resolv_conf_load (statp, NULL);
   else
     conf = __resolv_conf_get_current ();
   if (conf == NULL)
diff --git a/resolv/resolv_conf.c b/resolv/resolv_conf.c
index d954ba9a5a..bdd2ebb909 100644
--- a/resolv/resolv_conf.c
+++ b/resolv/resolv_conf.c
@@ -136,7 +136,7 @@  __resolv_conf_get_current (void)
     {
       /* Parse configuration while holding the lock.  This avoids
          duplicate work.  */
-      conf = __resolv_conf_load (NULL);
+      conf = __resolv_conf_load (NULL, NULL);
       if (conf != NULL)
         {
           if (global_copy->conf_current != NULL)
diff --git a/resolv/resolv_conf.h b/resolv/resolv_conf.h
index 01cbff9111..101e14bfe5 100644
--- a/resolv/resolv_conf.h
+++ b/resolv/resolv_conf.h
@@ -63,12 +63,16 @@  struct resolv_conf
    and the struct resolv_context facility.  */
 
 struct __res_state;
+struct file_change_detection;
 
 /* Read /etc/resolv.conf and return a configuration object, or NULL if
    /etc/resolv.conf cannot be read due to memory allocation errors.
-   If PREINIT is not NULL, some configuration values are taken from the
-   struct __res_state object.  */
-struct resolv_conf *__resolv_conf_load (struct __res_state *preinit)
+   If PREINIT is not NULL, some configuration values are taken from
+   the struct __res_state object.  If CHANGE is not null, file change
+   detection data is written to *CHANGE, based on the state of the
+   file after reading it.  */
+struct resolv_conf *__resolv_conf_load (struct __res_state *preinit,
+                                        struct file_change_detection *change)
   attribute_hidden __attribute__ ((warn_unused_result));
 
 /* Return a configuration object for the current /etc/resolv.conf