diff mbox series

[v2] support: Don't fail on fchown when spawning sgid processes

Message ID 20230531150003.3803030-1-siddhesh@sourceware.org
State Superseded
Headers
Series [v2] support: Don't fail on fchown when spawning sgid processes |

Checks

Context Check Description
redhat-pt-bot/TryBot-apply_patch success Patch applied to master at the time it was sent
redhat-pt-bot/TryBot-32bit success Build for i686
linaro-tcwg-bot/tcwg_glibc_build--master-aarch64 success Testing passed
linaro-tcwg-bot/tcwg_glibc_build--master-arm success Testing passed
linaro-tcwg-bot/tcwg_glibc_check--master-arm success Testing passed
linaro-tcwg-bot/tcwg_glibc_check--master-aarch64 pending Patch applied

Commit Message

Siddhesh Poyarekar May 31, 2023, 3 p.m. UTC 
  In some cases (e.g. when podman creates user containers), the only other
group assigned to the executing user is nobody and fchown fails with it
because the group is not mapped.  Do not fail the test in this case,
instead exit as unsupported.

Reported-by: Frederic Berat <fberat@redhat.com>
Signed-off-by: Siddhesh Poyarekar <siddhesh@sourceware.org>
---
 support/support_capture_subprocess.c | 15 +++++++++++++--
 1 file changed, 13 insertions(+), 2 deletions(-)

Comments

Andreas Schwab May 31, 2023, 3:13 p.m. UTC | #1 
On Mai 31 2023, Siddhesh Poyarekar via Libc-alpha wrote:

> diff --git a/support/support_capture_subprocess.c b/support/support_capture_subprocess.c
> index bae7d5fb20..3881e3610a 100644
> --- a/support/support_capture_subprocess.c
> +++ b/support/support_capture_subprocess.c
> @@ -153,9 +153,18 @@ copy_and_spawn_sgid (char *child_id, gid_t gid)
>  	  p += wrcount;
>  	}
>      }
> -  TEST_VERIFY (fchown (outfd, getuid (), gid) == 0);
> +
> +  int chowned = 0;

I think that can be bool.
Frederic Berat May 31, 2023, 3:56 p.m. UTC | #2 
On Wed, May 31, 2023 at 5:00 PM Siddhesh Poyarekar
<siddhesh@sourceware.org> wrote:
>
> In some cases (e.g. when podman creates user containers), the only other
> group assigned to the executing user is nobody and fchown fails with it
> because the group is not mapped.  Do not fail the test in this case,
> instead exit as unsupported.
>
> Reported-by: Frederic Berat <fberat@redhat.com>
> Signed-off-by: Siddhesh Poyarekar <siddhesh@sourceware.org>
> ---
>  support/support_capture_subprocess.c | 15 +++++++++++++--
>  1 file changed, 13 insertions(+), 2 deletions(-)
>
> diff --git a/support/support_capture_subprocess.c b/support/support_capture_subprocess.c
> index bae7d5fb20..3881e3610a 100644
> --- a/support/support_capture_subprocess.c
> +++ b/support/support_capture_subprocess.c
> @@ -153,9 +153,18 @@ copy_and_spawn_sgid (char *child_id, gid_t gid)
>           p += wrcount;
>         }
>      }
> -  TEST_VERIFY (fchown (outfd, getuid (), gid) == 0);
> +
> +  int chowned = 0;
> +  TEST_VERIFY ((chowned = fchown (outfd, getuid (), gid)) == 0
> +              || errno == EPERM);
>    if (support_record_failure_is_failed ())
>      goto err;
> +  else if (chowned != 0)
> +    {
> +      ret = 77;
> +      goto err;
> +    }
> +
>    TEST_VERIFY (fchmod (outfd, 02750) == 0);
>    if (support_record_failure_is_failed ())
>      goto err;
> @@ -192,8 +201,10 @@ err:
>        free (dirname);
>      }
>
> +  if (ret == 77)
> +    FAIL_UNSUPPORTED ("Failed to make sgid executable for test\n");
>    if (ret != 0)
> -    FAIL_EXIT1("Failed to make sgid executable for test\n");
> +    FAIL_EXIT1 ("Failed to make sgid executable for test\n");
>
>    return status;
>  }
> --
> 2.40.1
>

Looks good to me:
$> cat stdlib/tst-secure-getenv.test-result && cat stdlib/tst-secure-getenv.out
UNSUPPORTED: stdlib/tst-secure-getenv
original exit status 77
error: support_capture_subprocess.c:205: Failed to make sgid executable for test

Tested-by: Frédéric Bérat <fberat@redhat.com>
Siddhesh Poyarekar May 31, 2023, 4:07 p.m. UTC | #3 
On 2023-05-31 11:13, Andreas Schwab wrote:
> On Mai 31 2023, Siddhesh Poyarekar via Libc-alpha wrote:
> 
>> diff --git a/support/support_capture_subprocess.c b/support/support_capture_subprocess.c
>> index bae7d5fb20..3881e3610a 100644
>> --- a/support/support_capture_subprocess.c
>> +++ b/support/support_capture_subprocess.c
>> @@ -153,9 +153,18 @@ copy_and_spawn_sgid (char *child_id, gid_t gid)
>>   	  p += wrcount;
>>   	}
>>       }
>> -  TEST_VERIFY (fchown (outfd, getuid (), gid) == 0);
>> +
>> +  int chowned = 0;
> 
> I think that can be bool.
> 

Thanks, fixed in v3.

Sid
diff mbox series

Patch

diff --git a/support/support_capture_subprocess.c b/support/support_capture_subprocess.c
index bae7d5fb20..3881e3610a 100644
--- a/support/support_capture_subprocess.c
+++ b/support/support_capture_subprocess.c
@@ -153,9 +153,18 @@  copy_and_spawn_sgid (char *child_id, gid_t gid)
 	  p += wrcount;
 	}
     }
-  TEST_VERIFY (fchown (outfd, getuid (), gid) == 0);
+
+  int chowned = 0;
+  TEST_VERIFY ((chowned = fchown (outfd, getuid (), gid)) == 0
+	       || errno == EPERM);
   if (support_record_failure_is_failed ())
     goto err;
+  else if (chowned != 0)
+    {
+      ret = 77;
+      goto err;
+    }
+
   TEST_VERIFY (fchmod (outfd, 02750) == 0);
   if (support_record_failure_is_failed ())
     goto err;
@@ -192,8 +201,10 @@  err:
       free (dirname);
     }
 
+  if (ret == 77)
+    FAIL_UNSUPPORTED ("Failed to make sgid executable for test\n");
   if (ret != 0)
-    FAIL_EXIT1("Failed to make sgid executable for test\n");
+    FAIL_EXIT1 ("Failed to make sgid executable for test\n");
 
   return status;
 }