[PATCH 0/2] agent: Use npth_clock_gettime for S2K calibration.

classic Classic list List threaded Threaded
3 messages Options
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

[PATCH 0/2] agent: Use npth_clock_gettime for S2K calibration.

Damien Goutte-Gattat
Hi GnuPG folks,

This is a follow-up to the discussion on dev.gnupg.org/T3276.

The first of the following two patches proposes to replace the
use of times(2) (which leads to an infinite loop when used under
certain kernel configurations) by the npth_clock_gettime wrapper
as provided by the nPth library. It is quite non-intrusive and
is enough to fix the reported bug.

The second patch (built upon the first) is a proposal for a more
drastic change: it also replaces the Windows-specific code by the
same wrapper. This does not fix anything, the rationale here is
solely to somewhat simplify the code by abstracting system
differences behind the nPth wrapper.

(Note that I don't a have a Windows machine and therefore I
cannot test that second patch. All I can say is that the code
compiles under MingW.)

Damien Goutte-Gattat (2):
  agent: Use npth_clock_gettime for S2K calibration.
  agent: Use npth_clock_gettime on all platforms.

 agent/Makefile.am |  6 ++---
 agent/protect.c   | 74 +++++++------------------------------------------------
 2 files changed, 12 insertions(+), 68 deletions(-)

--
2.9.0


_______________________________________________
Gnupg-devel mailing list
[hidden email]
http://lists.gnupg.org/mailman/listinfo/gnupg-devel
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

[PATCH 1/2] agent: Use npth_clock_gettime for S2K calibration.

Damien Goutte-Gattat
* agent/protect.c (struct calibrate_time_s): Make that struct
an alias for timespec on non-W32 systems.
(calibrate_gettime): Use npth_clock_gettime instead of times.
(calibrate_elapsed_time): Adapt computation of elapsed time
accordingly.
* agent/Makefile.am: Link gpg-protect-tool against nPth.
--

It has been reported that the current method of measuring
elapsed times could result in the agent being stuck in an
infinite loop if the Linux kernel we are running on has been
configured with VIRT_CPU_ACCOUNTING_GEN=y.

Contrary to times(), clock_gettime() seems unaffected by
whatever happens in the kernel under that configuration.

We use the nPth wrapper for clock_gettime, which will provide
a fallback implementation on systems without clock_gettime.

GnuPG-bug-id: 3276
Signed-off-by: Damien Goutte-Gattat <[hidden email]>
---
 agent/Makefile.am |  6 +++---
 agent/protect.c   | 22 ++++++++++++----------
 2 files changed, 15 insertions(+), 13 deletions(-)

diff --git a/agent/Makefile.am b/agent/Makefile.am
index ce29462..ea6db2b 100644
--- a/agent/Makefile.am
+++ b/agent/Makefile.am
@@ -77,10 +77,10 @@ gpg_protect_tool_SOURCES = \
  protect-tool.c \
  protect.c cvt-openpgp.c
 
-gpg_protect_tool_CFLAGS = $(AM_CFLAGS) $(LIBASSUAN_CFLAGS) \
+gpg_protect_tool_CFLAGS = $(AM_CFLAGS) $(LIBASSUAN_CFLAGS) $(NPTH_CFLAGS) \
  $(INCICONV)
 gpg_protect_tool_LDADD = $(common_libs) $(LIBGCRYPT_LIBS) $(LIBASSUAN_LIBS) \
-         $(GPG_ERROR_LIBS) $(LIBINTL) $(NETLIBS) $(LIBICONV)
+         $(NPTH_LIBS) $(GPG_ERROR_LIBS) $(LIBINTL) $(NETLIBS) $(LIBICONV)
 
 gpg_preset_passphrase_SOURCES = \
  preset-passphrase.c
@@ -103,7 +103,7 @@ $(PROGRAMS): $(common_libs) $(commonpth_libs) $(pwquery_libs)
 TESTS = t-protect
 
 t_common_ldadd = $(common_libs)  $(LIBGCRYPT_LIBS) $(GPG_ERROR_LIBS) \
-          $(LIBINTL) $(LIBICONV) $(NETLIBS)
+          $(LIBINTL) $(LIBICONV) $(NETLIBS) $(NPTH_LIBS)
 
 t_protect_SOURCES = t-protect.c protect.c
 t_protect_LDADD = $(t_common_ldadd)
diff --git a/agent/protect.c b/agent/protect.c
index 18b44f1..42fb479 100644
--- a/agent/protect.c
+++ b/agent/protect.c
@@ -33,7 +33,7 @@
 # endif
 # include <windows.h>
 #else
-# include <sys/times.h>
+# include <npth.h>
 #endif
 
 #include "agent.h"
@@ -71,14 +71,14 @@ static const struct {
 
 
 /* A helper object for time measurement.  */
+#ifdef HAVE_W32_SYSTEM
 struct calibrate_time_s
 {
-#ifdef HAVE_W32_SYSTEM
   FILETIME creation_time, exit_time, kernel_time, user_time;
+};
 #else
-  clock_t ticks;
+ #define calibrate_time_s timespec
 #endif
-};
 
 
 static int
@@ -105,10 +105,7 @@ calibrate_get_time (struct calibrate_time_s *data)
                    &data->kernel_time, &data->user_time);
 # endif
 #else
-  struct tms tmp;
-
-  times (&tmp);
-  data->ticks = tmp.tms_utime;
+  npth_clock_gettime(data);
 #endif
 }
 
@@ -134,8 +131,13 @@ calibrate_elapsed_time (struct calibrate_time_s *starttime)
     return (unsigned long)((t2 - t1)/10000);
   }
 #else
-  return (unsigned long)((((double) (stoptime.ticks - starttime->ticks))
-                          /CLOCKS_PER_SEC)*10000000);
+  {
+    struct calibrate_time_s difftime;
+
+    npth_timersub(&stoptime, starttime, &difftime);
+
+    return difftime.tv_sec * 1000 + difftime.tv_nsec / 1000000.0;
+  }
 #endif
 }
 
--
2.9.0


_______________________________________________
Gnupg-devel mailing list
[hidden email]
http://lists.gnupg.org/mailman/listinfo/gnupg-devel
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

[PATCH 2/2] agent: Use npth_clock_gettime on all platforms.

Damien Goutte-Gattat
In reply to this post by Damien Goutte-Gattat
* agent/protect.c (struct calibrate_time_s): Removed.
(calibrate_get_time): Removed.
(calibrate_elapsed_time): Remove Windows-specific code.
(calibrate_s2k_count_one): Use npth_clock_gettime directly.
--

Since we are already using the npth_clock_gettime wrapper, we
no longer need a Windows-specific code path.  We can rely
entirely on nPth to abstract away the underlying system.

Signed-off-by: Damien Goutte-Gattat <[hidden email]>
---
 agent/protect.c | 74 +++++++--------------------------------------------------
 1 file changed, 8 insertions(+), 66 deletions(-)

diff --git a/agent/protect.c b/agent/protect.c
index 42fb479..08d5ca0 100644
--- a/agent/protect.c
+++ b/agent/protect.c
@@ -27,14 +27,7 @@
 #include <assert.h>
 #include <unistd.h>
 #include <sys/stat.h>
-#ifdef HAVE_W32_SYSTEM
-# ifdef HAVE_WINSOCK2_H
-#  include <winsock2.h>
-# endif
-# include <windows.h>
-#else
-# include <npth.h>
-#endif
+#include <npth.h>
 
 #include "agent.h"
 
@@ -70,17 +63,6 @@ static const struct {
 };
 
 
-/* A helper object for time measurement.  */
-#ifdef HAVE_W32_SYSTEM
-struct calibrate_time_s
-{
-  FILETIME creation_time, exit_time, kernel_time, user_time;
-};
-#else
- #define calibrate_time_s timespec
-#endif
-
-
 static int
 hash_passphrase (const char *passphrase, int hashalgo,
                  int s2kmode,
@@ -90,55 +72,15 @@ hash_passphrase (const char *passphrase, int hashalgo,
 
 
 
-/* Get the process time and store it in DATA.  */
-static void
-calibrate_get_time (struct calibrate_time_s *data)
-{
-#ifdef HAVE_W32_SYSTEM
-# ifdef HAVE_W32CE_SYSTEM
-  GetThreadTimes (GetCurrentThread (),
-                  &data->creation_time, &data->exit_time,
-                  &data->kernel_time, &data->user_time);
-# else
-  GetProcessTimes (GetCurrentProcess (),
-                   &data->creation_time, &data->exit_time,
-                   &data->kernel_time, &data->user_time);
-# endif
-#else
-  npth_clock_gettime(data);
-#endif
-}
-
-
 static unsigned long
-calibrate_elapsed_time (struct calibrate_time_s *starttime)
+calibrate_elapsed_time (struct timespec *starttime)
 {
-  struct calibrate_time_s stoptime;
-
-  calibrate_get_time (&stoptime);
-#ifdef HAVE_W32_SYSTEM
-  {
-    unsigned long long t1, t2;
-
-    t1 = (((unsigned long long)starttime->kernel_time.dwHighDateTime << 32)
-          + starttime->kernel_time.dwLowDateTime);
-    t1 += (((unsigned long long)starttime->user_time.dwHighDateTime << 32)
-           + starttime->user_time.dwLowDateTime);
-    t2 = (((unsigned long long)stoptime.kernel_time.dwHighDateTime << 32)
-          + stoptime.kernel_time.dwLowDateTime);
-    t2 += (((unsigned long long)stoptime.user_time.dwHighDateTime << 32)
-           + stoptime.user_time.dwLowDateTime);
-    return (unsigned long)((t2 - t1)/10000);
-  }
-#else
-  {
-    struct calibrate_time_s difftime;
+  struct timespec stoptime, difftime;
 
-    npth_timersub(&stoptime, starttime, &difftime);
+  npth_clock_gettime (&stoptime);
+  npth_timersub(&stoptime, starttime, &difftime);
 
-    return difftime.tv_sec * 1000 + difftime.tv_nsec / 1000000.0;
-  }
-#endif
+  return difftime.tv_sec * 1000 + difftime.tv_nsec / 1000000.0;
 }
 
 
@@ -149,9 +91,9 @@ calibrate_s2k_count_one (unsigned long count)
 {
   int rc;
   char keybuf[PROT_CIPHER_KEYLEN];
-  struct calibrate_time_s starttime;
+  struct timespec starttime;
 
-  calibrate_get_time (&starttime);
+  npth_clock_gettime (&starttime);
   rc = hash_passphrase ("123456789abcdef0", GCRY_MD_SHA1,
                         3, "saltsalt", count, keybuf, sizeof keybuf);
   if (rc)
--
2.9.0


_______________________________________________
Gnupg-devel mailing list
[hidden email]
http://lists.gnupg.org/mailman/listinfo/gnupg-devel
Loading...