[PATCH] UTIL: Ignore insufficient permission for some proc files

[Lukas Slebodnik]

ehlo,

Attached patch fixes problem with find-uid test on altlinux.
Alexey Shabalin reported this in another thread.

LS

[Stephen Gallagher]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 03/17/2014 04:02 AM, Lukas Slebodnik wrote:
>  ehlo,
>  
>  Attached patch fixes problem with find-uid test on altlinux. Alexey
>  Shabalin reported this in another thread.
>   

I've seen that intermittently when building with systemd in Koji as
well. I think there's a more fundamental problem here, and I'd
recommend not just papering over it. It's likely either a systemd bug
or a misunderstanding about how it interacts when building in a chroot.

> -----BEGIN PGP SIGNATURE-----
> Version: GnuPG v1
> Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/
>
> iEYEARECAAYFAlMm5RYACgkQeiVVYja6o6NbqwCfYje8QuRaYWZoEH6axryD3qji
> qFEAoIdf4Q/f2I0S2I8WX1+iGSVmOd7w
> =H+he
> -----END PGP SIGNATURE-----

[Lukas Slebodnik]

On (17/03/14 08:05), Stephen Gallagher wrote:
> On 03/17/2014 04:02 AM, Lukas Slebodnik wrote:
> > ehlo,
> > 
> > Attached patch fixes problem with find-uid test on altlinux. Alexey
> > Shabalin reported this in another thread.
> > 
>
>
> I've seen that intermittently when building with systemd in Koji as
> well. I think there's a more fundamental problem here, and I'd
> recommend not just papering over it. It's likely either a systemd bug
> or a misunderstanding about how it interacts when building in a chroot.
>  
proc fylesystem is not mount with option hidepid=1 on fedora.
maybe, you hit another bug.

LS

[Jakub Hrozek]

On Mon, Mar 17, 2014 at 01:32:39PM +0100, Lukas Slebodnik wrote:
>  On (17/03/14 08:05), Stephen Gallagher wrote:
>  >On 03/17/2014 04:02 AM, Lukas Slebodnik wrote:
>  >> ehlo,
>  >> 
>  >> Attached patch fixes problem with find-uid test on altlinux. Alexey
>  >> Shabalin reported this in another thread.
>  >> 
>  >
>  >
>  >I've seen that intermittently when building with systemd in Koji as
>  >well. I think there's a more fundamental problem here, and I'd
>  >recommend not just papering over it. It's likely either a systemd bug
>  >or a misunderstanding about how it interacts when building in a chroot.
>  >
>  proc fylesystem is not mount with option hidepid=1 on fedora.
>  maybe, you hit another bug.
>  
>  LS 
I was thinking about this on my way home and I wonder if we could wrap
Lukas's change in an #ifdef either based on a define passed during build
(-DSSS_PROC_HIDEPID) or automatically detected if possible. I agree with
Stephen that we shouldn't hide any potential issues by simply skipping
the EPERM error code.

[Lukas Slebodnik]

On (17/03/14 22:16), Jakub Hrozek wrote:
> On Mon, Mar 17, 2014 at 01:32:39PM +0100, Lukas Slebodnik wrote:
> > On (17/03/14 08:05), Stephen Gallagher wrote:
> > >On 03/17/2014 04:02 AM, Lukas Slebodnik wrote:
> > >> ehlo,
> > >>
> > >> Attached patch fixes problem with find-uid test on altlinux. Alexey
> > >> Shabalin reported this in another thread.
> > >>
> > >
> > >
> > >I've seen that intermittently when building with systemd in Koji as
> > >well. I think there's a more fundamental problem here, and I'd
> > >recommend not just papering over it. It's likely either a systemd bug
> > >or a misunderstanding about how it interacts when building in a chroot.
> > >
> > proc fylesystem is not mount with option hidepid=1 on fedora.
> > maybe, you hit another bug.
> >
> > LS
>
> I was thinking about this on my way home and I wonder if we could wrap
> Lukas's change in an #ifdef either based on a define passed during build
> (-DSSS_PROC_HIDEPID) or automatically detected if possible. I agree with
> Stephen that we shouldn't hide any potential issues by simply skipping
> the EPERM error code. 
I can reproduce failed test with "hidepid=1" only if I run find_uid-tests
as ordinary user. Test will pass if I run it as a root.

What kind of potential issues do you mean?

diff --git a/src/util/find_uid.c b/src/util/find_uid.c
index d34a4abd2c2bb4965ff49b9a750f387519544b0e..95991ebf2b14ff27a6a3740bfa048b5cfc631e94
100644
--- a/src/util/find_uid.c
+++ b/src/util/find_uid.c
@@ -223,6 +223,9 @@
static errno_t get_active_uid_linux(hash_table_t *table, uid_t search_uid)
         }

//snip

    while ((dirent = readdir(proc_dir)) != NULL) {
         if (only_numbers(dirent->d_name) != 0) continue;
         ret = name_to_pid(dirent->d_name, &pid);
         if (ret != EOK) {
             DEBUG(SSSDBG_CRIT_FAILURE, "name_to_pid failed.\n");
             goto done;
         }

         ret = get_uid_from_pid(pid, &uid);
         //Issue is logged in function get_uid_from_pid
         //DEBUG(SSSDBG_CRIT_FAILURE,
         //      "open failed [%d][%s].\n", error, strerror(error));

+        if (ret == EPERM) {
+            continue;
+        }

         if (ret != EOK) {
             DEBUG(1, ("get_uid_from_pid failed.\n"));
             goto done;


LS

[Lukas Slebodnik]

On (17/03/14 22:39), Lukas Slebodnik wrote:
> On (17/03/14 22:16), Jakub Hrozek wrote:
> >On Mon, Mar 17, 2014 at 01:32:39PM +0100, Lukas Slebodnik wrote:
> >> On (17/03/14 08:05), Stephen Gallagher wrote:
> >> >On 03/17/2014 04:02 AM, Lukas Slebodnik wrote:
> >> >> ehlo,
> >> >>
> >> >> Attached patch fixes problem with find-uid test on altlinux. Alexey
> >> >> Shabalin reported this in another thread.
> >> >>
> >> >
> >> >
> >> >I've seen that intermittently when building with systemd in Koji as
> >> >well. I think there's a more fundamental problem here, and I'd
> >> >recommend not just papering over it. It's likely either a systemd bug
> >> >or a misunderstanding about how it interacts when building in a chroot.
> >> >
> >> proc fylesystem is not mount with option hidepid=1 on fedora.
> >> maybe, you hit another bug.
> >>
> >> LS
> >
> >I was thinking about this on my way home and I wonder if we could wrap
> >Lukas's change in an #ifdef either based on a define passed during build
> >(-DSSS_PROC_HIDEPID) or automatically detected if possible. I agree with
> >Stephen that we shouldn't hide any potential issues by simply skipping
> >the EPERM error code.
>
> I can reproduce failed test with "hidepid=1" only if I run find_uid-tests
> as ordinary user. Test will pass if I run it as a root.
>
> What kind of potential issues do you mean?
>
> diff --git a/src/util/find_uid.c b/src/util/find_uid.c
> index d34a4abd2c2bb4965ff49b9a750f387519544b0e..95991ebf2b14ff27a6a3740bfa048b5cfc631e94
> 100644
> --- a/src/util/find_uid.c
> +++ b/src/util/find_uid.c
> @@ -223,6 +223,9 @@
> static errno_t get_active_uid_linux(hash_table_t *table, uid_t search_uid)
>          }
>
> //snip
>
>     while ((dirent = readdir(proc_dir)) != NULL) {
>          if (only_numbers(dirent->d_name) != 0) continue;
>          ret = name_to_pid(dirent->d_name, &pid);
>          if (ret != EOK) {
>              DEBUG(SSSDBG_CRIT_FAILURE, "name_to_pid failed.\n");
>              goto done;
>          }
>
>          ret = get_uid_from_pid(pid, &uid);
>          //Issue is logged in function get_uid_from_pid
>          //DEBUG(SSSDBG_CRIT_FAILURE,
>          //      "open failed [%d][%s].\n", error, strerror(error));
>
> +        if (ret == EPERM) {
> +            continue;
> +        }
>
>          if (ret != EOK) {
>              DEBUG(1, ("get_uid_from_pid failed.\n")); 
                   
^^^^^^^
            As you can see patch was formatted from 1-11 branch

LS

[Jakub Hrozek]

On Mon, Mar 17, 2014 at 10:39:07PM +0100, Lukas Slebodnik wrote:
>  On (17/03/14 22:16), Jakub Hrozek wrote:
>  >On Mon, Mar 17, 2014 at 01:32:39PM +0100, Lukas Slebodnik wrote:
>  >> On (17/03/14 08:05), Stephen Gallagher wrote:
>  >> >On 03/17/2014 04:02 AM, Lukas Slebodnik wrote:
>  >> >> ehlo,
>  >> >>
>  >> >> Attached patch fixes problem with find-uid test on altlinux. Alexey
>  >> >> Shabalin reported this in another thread.
>  >> >>
>  >> >
>  >> >
>  >> >I've seen that intermittently when building with systemd in Koji as
>  >> >well. I think there's a more fundamental problem here, and I'd
>  >> >recommend not just papering over it. It's likely either a systemd bug
>  >> >or a misunderstanding about how it interacts when building in a chroot.
>  >> >
>  >> proc fylesystem is not mount with option hidepid=1 on fedora.
>  >> maybe, you hit another bug.
>  >>
>  >> LS
>  >
>  >I was thinking about this on my way home and I wonder if we could wrap
>  >Lukas's change in an #ifdef either based on a define passed during build
>  >(-DSSS_PROC_HIDEPID) or automatically detected if possible. I agree with
>  >Stephen that we shouldn't hide any potential issues by simply skipping
>  >the EPERM error code.
>  
>  I can reproduce failed test with "hidepid=1" only if I run find_uid-tests
>  as ordinary user. Test will pass if I run it as a root. 
Build systems typically operate as a non-privileged user.

>  
>  What kind of potential issues do you mean? 
No particular issue, I'm just not a big fan of hiding an error.

[Lukas Slebodnik]

On (17/03/14 22:47), Jakub Hrozek wrote:
> On Mon, Mar 17, 2014 at 10:39:07PM +0100, Lukas Slebodnik wrote:
> > On (17/03/14 22:16), Jakub Hrozek wrote:
> > >On Mon, Mar 17, 2014 at 01:32:39PM +0100, Lukas Slebodnik wrote:
> > >> On (17/03/14 08:05), Stephen Gallagher wrote:
> > >> >On 03/17/2014 04:02 AM, Lukas Slebodnik wrote:
> > >> >> ehlo,
> > >> >>
> > >> >> Attached patch fixes problem with find-uid test on altlinux.
> Alexey
> > >> >> Shabalin reported this in another thread.
> > >> >>
> > >> >
> > >> >
> > >> >I've seen that intermittently when building with systemd in Koji as
> > >> >well. I think there's a more fundamental problem here, and I'd
> > >> >recommend not just papering over it. It's likely either a systemd
> bug
> > >> >or a misunderstanding about how it interacts when building in a chroot.
> > >> >
> > >> proc fylesystem is not mount with option hidepid=1 on fedora.
> > >> maybe, you hit another bug.
> > >>
> > >> LS
> > >
> > >I was thinking about this on my way home and I wonder if we could wrap
> > >Lukas's change in an #ifdef either based on a define passed during build
> > >(-DSSS_PROC_HIDEPID) or automatically detected if possible. I agree with
> > >Stephen that we shouldn't hide any potential issues by simply skipping
> > >the EPERM error code.
> >
> > I can reproduce failed test with "hidepid=1" only if I run find_uid-tests
> > as ordinary user. Test will pass if I run it as a root.
>
> Build systems typically operate as a non-privileged user.
>  
Test will pass in this case and it has no effect for a a normal running sssd.

> >
> > What kind of potential issues do you mean?
>
> No particular issue, I'm just not a big fan of hiding an error. 
And one more
time:

         Issue is logged in function get_uid_from_pid
         DEBUG(SSSDBG_CRIT_FAILURE,
               "open failed [%d][%s].\n", error, strerror(error));

In case of any problem, it would be easy to find a reason.

LS

[Lukas Slebodnik]

On (17/03/14 22:16), Jakub Hrozek wrote:
> On Mon, Mar 17, 2014 at 01:32:39PM +0100, Lukas Slebodnik wrote:
> > On (17/03/14 08:05), Stephen Gallagher wrote:
> > >On 03/17/2014 04:02 AM, Lukas Slebodnik wrote:
> > >> ehlo,
> > >> 
> > >> Attached patch fixes problem with find-uid test on altlinux. Alexey
> > >> Shabalin reported this in another thread.
> > >> 
> > >
> > >
> > >I've seen that intermittently when building with systemd in Koji as
> > >well. I think there's a more fundamental problem here, and I'd
> > >recommend not just papering over it. It's likely either a systemd bug
> > >or a misunderstanding about how it interacts when building in a chroot.
> > >
> > proc fylesystem is not mount with option hidepid=1 on fedora.
> > maybe, you hit another bug.
> > 
> > LS
>
> I was thinking about this on my way home and I wonder if we could wrap
> Lukas's change in an #ifdef either based on a define passed during build
> (-DSSS_PROC_HIDEPID) or automatically detected if possible. I agree with 
Compile
time detection is not a good idea.

Shaba wrote in another mail:
>  ALTLinux team usually  used "Hasher" for local build
> packages
>  It is local build environment, same on build server.
>  It is like sandman or mock(fedora)
>     Please see:
>    
> http://git.altlinux.org/people/ldv/packages/hasher.git?p=hasher.git;a=blo...
The test did not fail when I was building sssd in ALTLinux as a normal user.
The test only failed when "Hasher" was used.

Compile time detection will have the same effect like a patch in downstream.
If we do not want to skip proc files with insufficient permission we can leave
to downstream only.

> Stephen that we shouldn't hide any potential issues by simply
> skipping
> the EPERM error code. 
LS

[Jakub Hrozek]

On Mon, Mar 17, 2014 at 11:01:03PM +0100, Lukas Slebodnik wrote:
>  On (17/03/14 22:47), Jakub Hrozek wrote:
>  >On Mon, Mar 17, 2014 at 10:39:07PM +0100, Lukas Slebodnik wrote:
>  >> On (17/03/14 22:16), Jakub Hrozek wrote:
>  >> >On Mon, Mar 17, 2014 at 01:32:39PM +0100, Lukas Slebodnik wrote:
>  >> >> On (17/03/14 08:05), Stephen Gallagher wrote:
>  >> >> >On 03/17/2014 04:02 AM, Lukas Slebodnik wrote:
>  >> >> >> ehlo,
>  >> >> >>
>  >> >> >> Attached patch fixes problem with find-uid test on altlinux.
> Alexey
>  >> >> >> Shabalin reported this in another thread.
>  >> >> >>
>  >> >> >
>  >> >> >
>  >> >> >I've seen that intermittently when building with systemd in
> Koji as
>  >> >> >well. I think there's a more fundamental problem here, and
> I'd
>  >> >> >recommend not just papering over it. It's likely either a
> systemd bug
>  >> >> >or a misunderstanding about how it interacts when building in a
> chroot.
>  >> >> >
>  >> >> proc fylesystem is not mount with option hidepid=1 on fedora.
>  >> >> maybe, you hit another bug.
>  >> >>
>  >> >> LS
>  >> >
>  >> >I was thinking about this on my way home and I wonder if we could wrap
>  >> >Lukas's change in an #ifdef either based on a define passed during
> build
>  >> >(-DSSS_PROC_HIDEPID) or automatically detected if possible. I agree with
>  >> >Stephen that we shouldn't hide any potential issues by simply skipping
>  >> >the EPERM error code.
>  >>
>  >> I can reproduce failed test with "hidepid=1" only if I run
> find_uid-tests
>  >> as ordinary user. Test will pass if I run it as a root.
>  >
>  >Build systems typically operate as a non-privileged user.
>  >
>  Test will pass in this case and it has no effect for a a normal running sssd.
>  
>  >>
>  >> What kind of potential issues do you mean?
>  >
>  >No particular issue, I'm just not a big fan of hiding an error.
>  And one more time:
>  
>           Issue is logged in function get_uid_from_pid
>           DEBUG(SSSDBG_CRIT_FAILURE,
>                 "open failed [%d][%s].\n", error, strerror(error));
>  
>  In case of any problem, it would be easy to find a reason.
>  
>  LS 
Sorry this thread was stale for so long, more important issues kept
taking precedence.

Would we see this debug message during build? I thought (at least on
Fedora with recent automake) you'd only see PASS/FAIL.

[Jakub Hrozek]

On Mon, Apr 14, 2014 at 06:57:17PM +0200, Jakub Hrozek wrote:
>  On Mon, Mar 17, 2014 at 11:01:03PM +0100, Lukas Slebodnik wrote:
>  > On (17/03/14 22:47), Jakub Hrozek wrote:
>  > >On Mon, Mar 17, 2014 at 10:39:07PM +0100, Lukas Slebodnik wrote:
>  > >> On (17/03/14 22:16), Jakub Hrozek wrote:
>  > >> >On Mon, Mar 17, 2014 at 01:32:39PM +0100, Lukas Slebodnik wrote:
>  > >> >> On (17/03/14 08:05), Stephen Gallagher wrote:
>  > >> >> >On 03/17/2014 04:02 AM, Lukas Slebodnik wrote:
>  > >> >> >> ehlo,
>  > >> >> >>
>  > >> >> >> Attached patch fixes problem with find-uid test on
> altlinux. Alexey
>  > >> >> >> Shabalin reported this in another thread.
>  > >> >> >>
>  > >> >> >
>  > >> >> >
>  > >> >> >I've seen that intermittently when building with systemd
> in Koji as
>  > >> >> >well. I think there's a more fundamental problem here, and
> I'd
>  > >> >> >recommend not just papering over it. It's likely either a
> systemd bug
>  > >> >> >or a misunderstanding about how it interacts when building in
> a chroot.
>  > >> >> >
>  > >> >> proc fylesystem is not mount with option hidepid=1 on fedora.
>  > >> >> maybe, you hit another bug.
>  > >> >>
>  > >> >> LS
>  > >> >
>  > >> >I was thinking about this on my way home and I wonder if we could wrap
>  > >> >Lukas's change in an #ifdef either based on a define passed during
> build
>  > >> >(-DSSS_PROC_HIDEPID) or automatically detected if possible. I agree
> with
>  > >> >Stephen that we shouldn't hide any potential issues by simply
> skipping
>  > >> >the EPERM error code.
>  > >>
>  > >> I can reproduce failed test with "hidepid=1" only if I run
> find_uid-tests
>  > >> as ordinary user. Test will pass if I run it as a root.
>  > >
>  > >Build systems typically operate as a non-privileged user.
>  > >
>  > Test will pass in this case and it has no effect for a a normal running sssd.
>  > 
>  > >>
>  > >> What kind of potential issues do you mean?
>  > >
>  > >No particular issue, I'm just not a big fan of hiding an error.
>  > And one more time:
>  > 
>  >          Issue is logged in function get_uid_from_pid
>  >          DEBUG(SSSDBG_CRIT_FAILURE,
>  >                "open failed [%d][%s].\n", error, strerror(error));
>  > 
>  > In case of any problem, it would be easy to find a reason.
>  > 
>  > LS
>  
>  Sorry this thread was stale for so long, more important issues kept
>  taking precedence.
>  
>  Would we see this debug message during build? I thought (at least on
>  Fedora with recent automake) you'd only see PASS/FAIL. 
Replying to self...

sssd_be runs as root, so if we get EPERM in the backend, something is
really wrong.

What about ignoring EPERM in unit tests only as they run unprivileged?
What is the problem you see with this approach?

[Lukas Slebodnik]

On (14/04/14 19:44), Jakub Hrozek wrote:
> On Mon, Apr 14, 2014 at 06:57:17PM +0200, Jakub Hrozek wrote:
> > On Mon, Mar 17, 2014 at 11:01:03PM +0100, Lukas Slebodnik wrote:
> > > On (17/03/14 22:47), Jakub Hrozek wrote:
> > > >On Mon, Mar 17, 2014 at 10:39:07PM +0100, Lukas Slebodnik wrote:
> > > >> On (17/03/14 22:16), Jakub Hrozek wrote:
> > > >> >On Mon, Mar 17, 2014 at 01:32:39PM +0100, Lukas Slebodnik wrote:
> > > >> >> On (17/03/14 08:05), Stephen Gallagher wrote:
> > > >> >> >On 03/17/2014 04:02 AM, Lukas Slebodnik wrote:
> > > >> >> >> ehlo,
> > > >> >> >>
> > > >> >> >> Attached patch fixes problem with find-uid test on
> altlinux. Alexey
> > > >> >> >> Shabalin reported this in another thread.
> > > >> >> >>
> > > >> >> >
> > > >> >> >
> > > >> >> >I've seen that intermittently when building with
> systemd in Koji as
> > > >> >> >well. I think there's a more fundamental problem here,
> and I'd
> > > >> >> >recommend not just papering over it. It's likely
> either a systemd bug
> > > >> >> >or a misunderstanding about how it interacts when building
> in a chroot.
> > > >> >> >
> > > >> >> proc fylesystem is not mount with option hidepid=1 on fedora.
> > > >> >> maybe, you hit another bug.
> > > >> >>
> > > >> >> LS
> > > >> >
> > > >> >I was thinking about this on my way home and I wonder if we could
> wrap
> > > >> >Lukas's change in an #ifdef either based on a define passed
> during build
> > > >> >(-DSSS_PROC_HIDEPID) or automatically detected if possible. I agree
> with
> > > >> >Stephen that we shouldn't hide any potential issues by simply
> skipping
> > > >> >the EPERM error code.
> > > >>
> > > >> I can reproduce failed test with "hidepid=1" only if I run
> find_uid-tests
> > > >> as ordinary user. Test will pass if I run it as a root.
> > > >
> > > >Build systems typically operate as a non-privileged user.
> > > >
> > > Test will pass in this case and it has no effect for a a normal running sssd.
> > > 
> > > >>
> > > >> What kind of potential issues do you mean?
> > > >
> > > >No particular issue, I'm just not a big fan of hiding an error.
> > > And one more time:
> > > 
> > >          Issue is logged in function get_uid_from_pid
> > >          DEBUG(SSSDBG_CRIT_FAILURE,
> > >                "open failed [%d][%s].\n", error, strerror(error));
> > > 
> > > In case of any problem, it would be easy to find a reason.
> > > 
> > > LS
> > 
> > Sorry this thread was stale for so long, more important issues kept
> > taking precedence.
> > 
> > Would we see this debug message during build? I thought (at least on
> > Fedora with recent automake) you'd only see PASS/FAIL.
>
> Replying to self...
>
> sssd_be runs as root, so if we get EPERM in the backend, something is
> really wrong.
>
> What about ignoring EPERM in unit tests only as they run unprivileged?
> What is the problem you see with this approach? 
I will need to setup environment to test your approach.

BTW shaba has already the patch in downstream repo.
http://git.altlinux.org/people/shaba/packages/?p=sssd.git;a=summary

LS