[ruby-core:118874] Re: [Ruby master Bug#20586] Some filesystem calls in dir.c are missing error handling and can return incorrect results if interrupted

On 2024-08-16 21:59:04, "jeremyevans0 (Jeremy Evans) via ruby-core" <[e-mail]> spake thus:
>Issue #20586 has been updated by jeremyevans0 (Jeremy Evans).
>
>
>I've submitted a pull request that I hope will address most of these issues: https://github.com/ruby/ruby/pull/11393
[...]
>
>ivoanjo (Ivo Anjo) wrote:
>> I've spent quite some time looking at the `dir.c` sources, and here's the full list of APIs that suffer from issues:
[...]

>> Less sure about these:
[...]

>> * Error handling of `rb_home_dir_of` and `rb_default_home_dir` are a bit suspicious
>
>`rb_home_dir_of` I have switching to use `rb_getpwdirnam_for_login` in pull request 11202, and that function has proper error checks.  `rb_default_home_dir` calls `rb_getpwdirnam_for_login`, `rb_getlogin`, and `rb_getpwdiruid`.  Can you let me know what part of `rb_default_home_dir` still looks suspicious?
[...]

`rb_getpwdirnam_for_login` currently does not have any explicit
handling for `EINTR`. However, it /is/ being treated as an
error[0][1], so should never result in a false negative ("no record
found for the supplied login_name").

I think it could be made to retry on `EINTR`, so signals such as
`SIGPROF` triggering `EINTR` would not result in unnecessary
errors. An untested patch for this is included below.

Thanks,
-Al


[0] the same as if `EIO`, `ENOMEM`, `EMFILE`, or `ENFILE`

[1] Calls to either `getpwnam_r` or `getpwnam` that result in errno
     set to `EINTR` result in `rb_syserr_fail(...)`.


-->8--
diff --git a/process.c b/process.c
index b1f9931f06..a71951d496 100644
--- a/process.c
+++ b/process.c
@@ -5827,8 +5827,13 @@ rb_getpwdirnam_for_login(VALUE login_name)
      rb_str_set_len(getpwnm_tmp, bufsizenm);

      int enm;
+  again1:
      errno = 0;
      while ((enm = getpwnam_r(login, &pwdnm, bufnm, bufsizenm, &pwptr)) != 0) {
+        if (enm == EINTR) {
+            /* call was interrupted, e.g., by SIGPROF or similar */
+            goto again1;
+        }

          if (enm == ENOENT || enm== ESRCH || enm == EBADF || enm == EPERM) {
              /* not found; non-errors */
@@ -5859,16 +5864,23 @@ rb_getpwdirnam_for_login(VALUE login_name)

  # elif USE_GETPWNAM

+  again2:
      errno = 0;
      pwptr = getpwnam(login);
      if (pwptr) {
          /* found it */
          return rb_str_new_cstr(pwptr->pw_dir);
      }
-    if (errno
+    if (errno) {
+        if (errno == EINTR) {
+            /* call was interrupted, e.g., by SIGPROF or similar */
+            goto again2;
+        }
+
          /*   avoid treating as errors errno values that indicate "not found" */
-        && ( errno != ENOENT && errno != ESRCH && errno != EBADF && errno != EPERM)) {
-        rb_syserr_fail(errno, "getpwnam");
+        if ( errno != ENOENT && errno != ESRCH && errno != EBADF && errno != EPERM) {
+            rb_syserr_fail(errno, "getpwnam");
+        }
      }

      return Qnil;  /* not found */
--8<--

-- 
a l a n   d.   s a l e w s k i
[e-mail]
[e-mail]
https://github.com/salewski
 ______________________________________________
 ruby-core mailing list -- [e-mail]
 To unsubscribe send an email to [e-mail]
 ruby-core info -- https://ml.ruby-lang.org/mailman3/lists/ruby-core.ml.ruby-lang.org/