[PATCH] virmockstathelpers: Load aliases for 64-bit time

Michal Privoznik posted 1 patch 1 week, 6 days ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/libvirt tags/patchew/0bd7ca3e0d8716e53401533e73d0ce51e644e20a.1669030983.git.mprivozn@redhat.com
tests/virmockstathelpers.c | 12 ++++++++++++
1 file changed, 12 insertions(+)
[PATCH] virmockstathelpers: Load aliases for 64-bit time
Posted by Michal Privoznik 1 week, 6 days ago
On 32-bit arches, it's possible not only to request
-D_FILE_OFFSET_BITS=64 (which is always done with meson) but also
-D_TIME_BITS=64. With glibc, both of these affect what variant of
stat() or lstat() is called. With 64 bit time it's:
__stat64_time64() or __lstat64_time64(), respectively.

Fortunately, no other variant (__xstat(), __xstat64()) has
_time64 alternative and thus does not need similar treatment.

Similarly, musl is not affected by this.

Resolves: https://gitlab.com/libvirt/libvirt/-/issues/404
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
---

Mind you, after this I still see two tests failing:

  1) virnettlscontexttest
  2) virnettlssessiontest

But I believe the problem is that GnuTLS on my 32bit RPi was not built
with -D_TIME_BITS=64 and thus when generating certificates and setting
their activation/expiry times I get the following errors:

TEST: virnettlscontexttest
 1) TLS Context cacertreq.filename + servercertreq.filename           ... libvirt: XML-RPC error : The server certificate servercertreq-ctx.pem is not yet active
FAILED

TEST: virnettlssessiontest
 1) TLS Session servercertreq.filename + clientcertreq.filename       ... libvirt: XML-RPC error : authentication failed: Failed to verify peer's certificate
FAILED

Setting of activation/expiry times is done by calling:

  1) gnutls_x509_crt_set_activation_time(..., time_t act_time)
  2) gnutls_x509_crt_set_expiration_time(..., time_t exp_time)

And this is how calling the first looks under disassembly:

    7428:       e51b1128        ldr     r1, [fp, #-296] ; 0xfffffed8
    742c:       e14b2dd4        ldrd    r2, [fp, #-212] ; 0xffffff2c
    7430:       e1a00001        mov     r0, r1
    7434:       ebfff23d        bl      3d30 <gnutls_x509_crt_set_activation_time@plt>

Now, ldrd instruction modifies both r2 and r3 by loading 4+4 bytes from
a memory address [1]. But the first thing that
gnutls_x509_crt_set_activation_time() does is overwriting r3 register:

000fb4a0 <gnutls_x509_crt_set_activation_time>:
   fb4a0:       e52de004        push    {lr}            ; (str lr, [sp, #-4]!)
   fb4a4:       e250c000        subs    ip, r0, #0
   fb4a8:       e59f3080        ldr     r3, [pc, #128]  ; fb530 <gnutls_x509_crt_set_activation_time+0x90>
   fb4ac:       e24dd00c        sub     sp, sp, #12
   fb4b0:       e08f3003        add     r3, pc, r3
   fb4b4:       0a000009        beq     fb4e0 <gnutls_x509_crt_set_activation_time+0x40>


With 32-bit time_t the disassembly looks a bit different:

    73e4:       e51b3114        ldr     r3, [fp, #-276] ; 0xfffffeec
    73e8:       e51b10f4        ldr     r1, [fp, #-244] ; 0xffffff0c
    73ec:       e1a00003        mov     r0, r3
    73f0:       ebfff250        bl      3d38 <gnutls_x509_crt_set_activation_time@plt>


1: It should be 'ldrd r2, r3, $memAddr' but the second register is
implied by the first one, so it can be shortened:

https://developer.arm.com/documentation/ddi0406/c/Application-Level-Architecture/Instruction-Details/Alphabetical-list-of-instructions/LDRD--immediate-

 tests/virmockstathelpers.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/tests/virmockstathelpers.c b/tests/virmockstathelpers.c
index 39e270eaac..5b1f3b08a7 100644
--- a/tests/virmockstathelpers.c
+++ b/tests/virmockstathelpers.c
@@ -54,6 +54,10 @@
  * Unfortunately, because we are trying to mock replace the C library,
  * we need to know about this internal impl detail.
  *
+ * Furthermore, support for 64-bit time can be enabled, which on 32-bit
+ * systems with glibc overwrites stat64() to __stat64_time64() and lstat64()
+ * to __lstat64_time64().
+ *
  * On macOS stat() and lstat() are resolved to _stat$INODE64 and
  * _lstat$INODE64, respectively. stat(2) man page also declares that
  * stat64(), lstat64() and fstat64() are deprecated, and when
@@ -168,7 +172,11 @@ static void virMockStatInit(void)
     fdebug("real stat %p\n", real_stat);
 #endif
 #ifdef MOCK_STAT64
+# if defined(__GLIBC__) && defined(_TIME_BITS) && _TIME_BITS == 64
+    VIR_MOCK_REAL_INIT_ALIASED(stat64, "__stat64_time64");
+# else
     VIR_MOCK_REAL_INIT(stat64);
+# endif
     fdebug("real stat64 %p\n", real_stat64);
 #endif
 #ifdef MOCK___XSTAT
@@ -188,7 +196,11 @@ static void virMockStatInit(void)
     fdebug("real lstat %p\n", real_lstat);
 #endif
 #ifdef MOCK_LSTAT64
+# if defined(__GLIBC__) && defined(_TIME_BITS) && _TIME_BITS == 64
+    VIR_MOCK_REAL_INIT_ALIASED(lstat64, "__lstat64_time64");
+# else
     VIR_MOCK_REAL_INIT(lstat64);
+# endif
     fdebug("real lstat64 %p\n", real_lstat64);
 #endif
 #ifdef MOCK___LXSTAT
-- 
2.37.4
Re: [PATCH] virmockstathelpers: Load aliases for 64-bit time
Posted by Ján Tomko 1 week, 6 days ago
On a Monday in 2022, Michal Privoznik wrote:
>On 32-bit arches, it's possible not only to request
>-D_FILE_OFFSET_BITS=64 (which is always done with meson) but also
>-D_TIME_BITS=64. With glibc, both of these affect what variant of
>stat() or lstat() is called. With 64 bit time it's:
>__stat64_time64() or __lstat64_time64(), respectively.
>
>Fortunately, no other variant (__xstat(), __xstat64()) has
>_time64 alternative and thus does not need similar treatment.
>
>Similarly, musl is not affected by this.
>
>Resolves: https://gitlab.com/libvirt/libvirt/-/issues/404

I can't load that link :)

>Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
>---
>

Reviewed-by: Ján Tomko <jtomko@redhat.com>

Jano