Re: [v2] nsswitch: do not reload if "/" changes

[Florian Weimer <fweimer@redhat.com>]

* DJ Delorie:

> Florian Weimer <fweimer@redhat.com> writes:
>> I still think you should disable (re)loading if __stat64 fails.
>
> Ok.
>
>> Maybe call __nss_module_disable_loading after releasing local->lock?
>
> Er, I already do?
>
> +      __libc_lock_unlock (local->lock);
> +      __nss_module_disable_loading ();

Ah, right.  I was confused.

> +  /* Before we reload, verify that "/" hasn't changed.  We assume that
> +     errors here are very unlikely, but the chance that we're entering
> +     a container is also very unlikely, so we err on the side of both
> +     very unlikely things not happening at the same time.  */
> +  if (__stat64 ("/", &str) != 0
> +      || (local->root_ino != 0
> +	  && (str.st_ino != local->root_ino
> +	      ||  str.st_dev != local->root_dev)))
> +    {
> +      /* Change detected; disable reloading.  */
> +      atomic_store_release (&local->data.reload_disabled, 1);
> +      __libc_lock_unlock (local->lock);
> +      __nss_module_disable_loading ();
> +      return true;
> +    }

0 used to be a valid inode number for XFS, but I suspect a lot will break if /
uses it.

There are some style issues:

+   GRP(4), /* match */
+  
+  pw = getpwnam("test1");
+  gr = getgrnam("name4");
+  chdir("/");
+  pw = getpwnam("test2");
+  gr = getgrnam("test3");
+  pw = getpwnam("test1");

Comment formatting, trailing whitespace, missing space before '('.

Rest looks okay.  I assume the test fails in the expected way without
the patch. 8-)

Thanks,
Florian
-- 
Red Hat GmbH, https://de.redhat.com/ , Registered seat: Grasbrunn,
Commercial register: Amtsgericht Muenchen, HRB 153243,
Managing Directors: Charles Cachera, Brian Klemm, Laurie Krebs, Michael O'Neill