gramine icon indicating copy to clipboard operation
gramine copied to clipboard

[LibOS] time zone is incorrect in syscall `gettimeofday()` or glibc `localtime()` or bash command `date`

Open llly opened this issue 3 years ago • 16 comments

Description of the problem

Current Gramine doesn't take care of time zone. The requests for time with time zone is incorrect and inconsistent.

Steps to reproduce

  1. Choose time zone using timedatectl set-timezone on Ubuntu.
  2. execute sample
time_t timep;
time (&timep);
printf("%s\n",asctime(localtime(&timep)));

struct timeval tv;
struct timezone tz= {60, 1};
gettimeofday (&tv , &tz);
printf("tz_minuteswest: %d\n", tz.tz_minuteswest);
  1. execute bash -c date in CI-Example.

Expected results

Mon Mar 21 14:39:45 2022
tz_minuteswest: -480
Mon Mar 21 14:40:21 CST 2022

Actual results

Mon Mar 21 06:40:45 2022
tz_minuteswest: 60
Mon Mar 21 06:41:41 UTC 2022

Additional information

We can see that all time become UTC time, while tz_minuteswest doesn't reflect UTC time.

Gramine commit hash

e37f773d4d910799a854b90334f2ff8eb6f411bd

llly avatar Mar 21 '22 07:03 llly

If you add this to CI-Example bash manifest.template, the result is correct:

...
loader.env.TZ = ":/etc/localtime"
...
fs.mount.etc.type = "chroot"
fs.mount.etc.path = "/etc"
fs.mount.etc.uri = "file:/etc"
...
sgx.allowed_files = [
...
  "file:/etc/localtime",
]

Similar happens to the C example.

However, tz_minuteswest is always 60. Not sure why.

lejunzhu avatar Mar 21 '22 08:03 lejunzhu

Looking at shim_do_gettimeofday(), tz is never touched, hence the initial value 60 is always there. Is it because this argument is obsolete or a bug?

lejunzhu avatar Mar 21 '22 09:03 lejunzhu

loader.env.TZ = ":/etc/localtime"

Yes, The workaround works for localtime and date. Native app doesn't need a TZ env to get time zone, OS doesn't provide TZ env, neither.

llly avatar Mar 22 '22 03:03 llly

loader.env.TZ = ":/etc/localtime"

Yes, The workaround works for localtime and date. Native app doesn't need a TZ env to get time zone, OS doesn't provide TZ env, neither.

TZ env is only needed for the bash -c case. C sample code runs fine without it.

If TZ is not set, when using "bash -c date", it will try to open /etc/localtime, e.g. /usr/local/etc/localtime. This doesn't seem correct...

lejunzhu avatar Mar 22 '22 11:03 lejunzhu

The root cause is the glibc comes with Gramine has a different TZDEFAULT macro:

$ strings /usr/local/lib/x86_64-linux-gnu/gramine/runtime/glibc/libc.so  | grep "/usr/local/etc"
/usr/local/etc/localtime

This is introduced in glibc Makeconfig:

...
sysconfdir = $(prefix)/etc
...
localtime-file = $(sysconfdir)/localtime

So there are 3 options to get the correct timezone: 1. set TZ environment variable in the manifest, 2. copy /etc/localtime to /etc/localtime 3. change the setting of sysconfdir when building glibc.

lejunzhu avatar Mar 23 '22 01:03 lejunzhu

@lejunzhu @llly Looks like we have two separate issues here:

  1. Time syscalls ignore the struct timezone argument completely, e.g. https://github.com/gramineproject/gramine/blob/9eafa5f7731650dc54043e4aa4804b6a2e34e292/LibOS/shim/src/sys/shim_time.c#L14

  2. Gramine's patched Glibc needs special enablement in the manifest file to mimic the exact same behavior as system-wide Glibc, in terms of date-time-timezone (but not in terms of tz_minuteswest).

For the first issue, is this anything critical? From what I see, timezone::tz_minuteswest is brittle and not recommended for use. I doubt any real-world application actually uses it. So I would vote for Gramine to continue ignoring this argument.

For the second issue, I would prefer our patched Glibc to be built in the same way as the system-wide Glibc is built.

  • In other words, I don't like the idea of adding loader.env.TZ + sgx.allowed_files = ["file:/etc/localtime"]. This is magic (especially the TZ envvar), and the users shouldn't know/apply this. Gramine's Glibc should be a complete drop-in replacement for the system-wide Glibc.
  • So I vote for @lejunzhu's proposal number 3.

@lejunzhu Do you know why sysconfdir in the Glibc Makefile has a wrong prefix? And what would be a simple way of fixing this prefix (it looks like this prefix can be simply empty-string, this should work for all our supported distros -- Debian/Ubuntu and CentOS/RHEL/Fedora)?

@woju @pwmarcz @mkow Any comments on the second issue?

dimakuv avatar Apr 04 '22 06:04 dimakuv

@lejunzhu Do you know why sysconfdir in the Glibc Makefile has a wrong prefix? And what would be a simple way of fixing this prefix (it looks like this prefix can be simply empty-string, this should work for all our supported distros -- Debian/Ubuntu and CentOS/RHEL/Fedora)?

This is because glibc's Makeconfig will prepend $prefix to sysconfdir by default:

sysconfdir = $(prefix)/etc

And we set $prefix in compile.sh at https://github.com/gramineproject/gramine/blob/e37f773d4d910799a854b90334f2ff8eb6f411bd/subprojects/packagefiles/glibc-2.34/compile.sh#L53

But I don't know how to quickly fix it. Actually, if I set "/home/daniel/local" as Gramine prefix, I will find about 10 places in libc.so pointing to this directory:

$ strings libc.so | grep daniel
/home/daniel/local/lib/x86_64-linux-gnu/gramine/runtime/glibc/locale
/home/daniel/local/lib/x86_64-linux-gnu/gramine/runtime/glibc/locale/locale-archive
/home/daniel/local/share/locale
/home/daniel/local/share/zoneinfo
/home/daniel/local/lib/x86_64-linux-gnu/gramine/runtime/glibc/gconv/gconv-modules.cache
/home/daniel/local/share/locale
/home/daniel/local/share/locale/%L/%N:/home/daniel/local/share/locale/%L/LC_MESSAGES/%N:/home/daniel/local/share/locale/%l/%N:/home/daniel/local/share/locale/%l/LC_MESSAGES/%N:
/home/daniel/local/etc/localtime
/home/daniel/local/libexec/getconf
/home/daniel/local/lib/x86_64-linux-gnu/gramine/runtime/glibc/gcal/share/locale/%L/%N:/home/daniel/local/share/locale/%L/LC_MESSAGES/%N:/home/daniel/local/share/locale/%l/%N:/home/daniel/local/share/locale/%l/LC_MESSAGES/%N:
/home/daniel/local/lib/ld-linux-x86-64.so.2

All these paths look suspicious and probably can't be found when running inside Gramine. Maybe we should specify $install_root instead of $prefix when building and installing glibc?

lejunzhu avatar Apr 04 '22 09:04 lejunzhu

From what I read, prefix=/usr is the correct way to build Glibc that will use system-wide configuration files: https://sourceware.org/glibc/wiki/Testing/Builds#:~:text=Building%20glibc%20without%20installing,-To%20build%20glibc&text=By%20building%20with%20the%20prefix,prefix%20for%20a%20system%20glibc.

@woju @pwmarcz @mkow Summoning you.

dimakuv avatar Apr 04 '22 10:04 dimakuv

configure --sysconfdir=/etc should work, if you need to override to to whatever, especially outside prefix. Makeconfig setting is in ifndef, and is a default. There are other overrides, and if we want to use system's paths for timezone database, we could override --datadir= or even --zonedir= (which defaults to $datadir/zoneinfo).

If we really care, other settings should be considered (now or sometime in the future), like --localedir, which is the other part of --datadir=.

@dimakuv In the wiki page you linked, I think they simply forgot about /etc.

But I don't understand why we need to mimic system's glibc 1:1, because we're really an alternative OS and alternative distribution. We should be compatible, yes, but we're not under obligation to copy everything.

Yes, TZ= is an ugly hack, it belongs to user, not distributors, and should be disposed of.

woju avatar Apr 04 '22 12:04 woju

configure --sysconfdir=/etc should work

Then I vote for this! Nice and contained.

But I don't understand why we need to mimic system's glibc 1:1, because we're really an alternative OS and alternative distribution. We should be compatible, yes, but we're not under obligation to copy everything.

Well, we debate on this for the last 4-5 years :) Glibc is not a stand-alone library, it depends on a bunch of scattered text files (like this /etc/localtime) and opens them from time to time. So we have two options:

  1. Pack and ship these files together with our patched Glibc, and build Glibc so that it points to these files.
  2. Allow our patched Glibc to use the system-wide files.

We currently use option 2, but simply because it is easier (just tinker with the manifest file). And with this GitHub issue, we see that at least sometimes, our patched Glibc doesn't do option 1 nor option 2 by default, but it defaults to something totally meaningless.

So since we're currently doing option 2, we should do it uniformly.

dimakuv avatar Apr 04 '22 12:04 dimakuv

Actually, why do we configure Glibc --prefix="$PREFIX"? We're compiling glibc to be used inside Gramine, and a host path like /home/daniel/local/ (or whatever you end up installing Gramine on host) does not make any sense there.

As a user, I would expect the glibc in Gramine to use paths like /etc/localtime, without any extra prefixes. Remember, this is the main runtime in our "distribution", not some locally-installed alternative library.

pwmarcz avatar Apr 04 '22 13:04 pwmarcz

Because we can map allowed files 1:1 and nothing should break. We'd have to document a canonical mount that should be expected in compatible manifests. We probably can change the approach if you think that would be better.

woju avatar Apr 04 '22 13:04 woju

Because we can map allowed files 1:1 and nothing should break.

What do you mean? For this to make sense, we would have to mount Gramine install dir under the same path inside Gramine. AFAICT, we never do that, we only mount the Gramine install dir under /lib.

pwmarcz avatar Apr 04 '22 13:04 pwmarcz

If we don't do that and it still works, then I suppose there isn't any reason.

woju avatar Apr 04 '22 16:04 woju

Currently, I can use

fs.mounts = [
{ path = "/usr/local/etc", uri = "file:/etc" },
]

to workaround this issue until prefix of Glibc updated.

llly avatar Apr 11 '22 01:04 llly

Sorry, if this offtopic or something... I don't know this project here, but I came across this thread, when troubleshooting a similar issue in my toolchain. I was able to fix it like this: echo "localtime-file = /etc/localtime" > $glibc/configparms Where $glibc is the glibc source folder. And this is run prior to configuring glibc.

hinkelski avatar Jul 05 '23 08:07 hinkelski