Message ID | 95b81fd1ccd9d41c4f6a897a1c43d1298da0486e.1579631655.git.fweimer@redhat.com |
---|---|
State | New |
Headers | show |
Series | Race condition in /etc/resolv.conf reloading (bug 25420) | expand |
On 21/01/2020 15:42, Florian Weimer wrote: > __resolv_conf_get_current should only record the initial file > change data if after verifying that file just read matches the > original measurement. Fixes commit aef16cc8a4c670036d45590877 > ("resolv: Automatically reload a changed /etc/resolv.conf file > [BZ #984]"). LGTM, thanks. Reviewed-by: Adhemerval Zanella <adhemerval.zanella@linaro.org> > --- > resolv/resolv_conf.c | 19 +++++++++++++------ > 1 file changed, 13 insertions(+), 6 deletions(-) > > diff --git a/resolv/resolv_conf.c b/resolv/resolv_conf.c > index bdd2ebb909..29a1f4fb94 100644 > --- a/resolv/resolv_conf.c > +++ b/resolv/resolv_conf.c > @@ -136,18 +136,25 @@ __resolv_conf_get_current (void) > { > /* Parse configuration while holding the lock. This avoids > duplicate work. */ > - conf = __resolv_conf_load (NULL, NULL); > + struct file_change_detection after_load; > + conf = __resolv_conf_load (NULL, &after_load); > if (conf != NULL) > { > if (global_copy->conf_current != NULL) > conf_decrement (global_copy->conf_current); > global_copy->conf_current = conf; /* Takes ownership. */ > > - /* Update file modification stamps. The configuration we > - read could be a newer version of the file, but this does > - not matter because this will lead to an extraneous reload > - later. */ > - global_copy->file_resolve_conf = initial; > + /* Update file change detection data, but only if it matches > + the initial measurement. This avoids an ABA race in case > + /etc/resolv.conf is temporarily replaced while the file > + is read (after the initial measurement), and restored to > + the initial version later. */ > + if (file_is_unchanged (&initial, &after_load)) > + global_copy->file_resolve_conf = after_load; > + else > + /* If there is a discrepancy, trigger a reload during the > + next use. */ > + global_copy->file_resolve_conf.size = -1; > } > } > > Ok.
diff --git a/resolv/resolv_conf.c b/resolv/resolv_conf.c index bdd2ebb909..29a1f4fb94 100644 --- a/resolv/resolv_conf.c +++ b/resolv/resolv_conf.c @@ -136,18 +136,25 @@ __resolv_conf_get_current (void) { /* Parse configuration while holding the lock. This avoids duplicate work. */ - conf = __resolv_conf_load (NULL, NULL); + struct file_change_detection after_load; + conf = __resolv_conf_load (NULL, &after_load); if (conf != NULL) { if (global_copy->conf_current != NULL) conf_decrement (global_copy->conf_current); global_copy->conf_current = conf; /* Takes ownership. */ - /* Update file modification stamps. The configuration we - read could be a newer version of the file, but this does - not matter because this will lead to an extraneous reload - later. */ - global_copy->file_resolve_conf = initial; + /* Update file change detection data, but only if it matches + the initial measurement. This avoids an ABA race in case + /etc/resolv.conf is temporarily replaced while the file + is read (after the initial measurement), and restored to + the initial version later. */ + if (file_is_unchanged (&initial, &after_load)) + global_copy->file_resolve_conf = after_load; + else + /* If there is a discrepancy, trigger a reload during the + next use. */ + global_copy->file_resolve_conf.size = -1; } }