[PATCH v3 0/3] time namespace aware system boot time

[PATCH v3 0/3] time namespace aware system boot time

[Michael Weiß]

Time namespaces make it possible to virtualize time inside of
containers, e.g., it is feasible to reset the uptime of a container
to zero by setting the time namespace offset for boottime to the
negated current value of the CLOCK_BOOTTIME.

However, the boot time stamp provided by getboottime64() does not
take care of time namespaces. The resulting boot time stamp 'btime'
provided by /proc/stat does not show a plausible time stamp inside
the time namespace of a container.

We address this by shifting the value returned by getboottime64()
by subtracting the boottime offset of the time namespace.
(A selftest to check the expected /proc/stat 'btime' inside the
namespace is provided.)

Further, to avoid to show processes as time travelers inside of the
time namespace the boottime offset then needs to be added to the
start_bootime provided by the task_struct.

v3 Changes:
leave getboottime64() unchanged and shift the boot timestamp in
'fs/proc/stat.c' as result of the discussion with Andrei and Thomas.

v2 Changes:
Fixed compile errors with TIME_NS not set in config
Reported-by: kernel test robot <lkp@intel.com>

Michael Weiß (3):
  timens: additional helper function to add boottime in nsec
  fs/proc: apply the time namespace offset to /proc/stat btime
  selftests/timens: added selftest for /proc/stat btime

 fs/proc/array.c                         |  6 ++-
 fs/proc/stat.c                          | 17 +++++++-
 include/linux/time_namespace.h          | 13 ++++++
 tools/testing/selftests/timens/procfs.c | 58 ++++++++++++++++++++++++-
 4 files changed, 90 insertions(+), 4 deletions(-)

-- 
2.20.1

[PATCH v3 1/3] timens: additional helper function to add boottime in nsec

[Michael Weiß]

Provide a helper function to apply the boottime offset to u64 types
in nanoseconds.

Signed-off-by: Michael Weiß <michael.weiss@aisec.fraunhofer.de>
---
 include/linux/time_namespace.h | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/include/linux/time_namespace.h b/include/linux/time_namespace.h
index 5b6031385db0..5372181010f9 100644
--- a/include/linux/time_namespace.h
+++ b/include/linux/time_namespace.h
@@ -77,6 +77,13 @@ static inline void timens_add_boottime(struct timespec64 *ts)
 	*ts = timespec64_add(*ts, ns_offsets->boottime);
 }
 
+static inline u64 timens_add_boottime_ns(u64 nsec)
+{
+	struct timens_offsets *ns_offsets = &current->nsproxy->time_ns->offsets;
+
+	return nsec + timespec64_to_ns(&ns_offsets->boottime);
+}
+
 ktime_t do_timens_ktime_to_host(clockid_t clockid, ktime_t tim,
 				struct timens_offsets *offsets);
 
@@ -130,6 +137,12 @@ static inline int timens_on_fork(struct nsproxy *nsproxy,
 
 static inline void timens_add_monotonic(struct timespec64 *ts) { }
 static inline void timens_add_boottime(struct timespec64 *ts) { }
+
+static inline u64 timens_add_boottime_ns(u64 nsec)
+{
+	return nsec;
+}
+
 static inline ktime_t timens_ktime_to_host(clockid_t clockid, ktime_t tim)
 {
 	return tim;
-- 
2.20.1

[PATCH v3 2/3] fs/proc: apply the time namespace offset to /proc/stat btime

[Michael Weiß]

'/proc/stat' provides the field 'btime' which states the time stamp of
system boot in seconds. In case of time namespaces, the offset to the
boot time stamp was not applied earlier. However, in container
runtimes which utilize time namespaces to virtualize boottime of a
container, this leaks information about the host system boot time.

Therefore, we make procfs to virtualize also the btime field by
subtracting the offset of the timens boottime from 'btime' before
printing the stats.

Since start_boottime of processes are seconds since boottime and the
boottime stamp is now shifted according to the timens offset, the
offset of the time namespace also needs to be applied before the
process stats are given to userspace.

This avoids that processes shown, e.g., by 'ps' appear as time
travelers in the corresponding time namespace.

Signed-off-by: Michael Weiß <michael.weiss@aisec.fraunhofer.de>
---
 fs/proc/array.c |  6 ++++--
 fs/proc/stat.c  | 17 ++++++++++++++++-
 2 files changed, 20 insertions(+), 3 deletions(-)

diff --git a/fs/proc/array.c b/fs/proc/array.c
index 65ec2029fa80..277f654f289e 100644
--- a/fs/proc/array.c
+++ b/fs/proc/array.c
@@ -56,6 +56,7 @@
 #include <linux/types.h>
 #include <linux/errno.h>
 #include <linux/time.h>
+#include <linux/time_namespace.h>
 #include <linux/kernel.h>
 #include <linux/kernel_stat.h>
 #include <linux/tty.h>
@@ -533,8 +534,9 @@ static int do_task_stat(struct seq_file *m, struct pid_namespace *ns,
 	priority = task_prio(task);
 	nice = task_nice(task);
 
-	/* convert nsec -> ticks */
-	start_time = nsec_to_clock_t(task->start_boottime);
+	/* apply timens offset for boottime and convert nsec -> ticks */
+	start_time =
+		nsec_to_clock_t(timens_add_boottime_ns(task->start_boottime));
 
 	seq_put_decimal_ull(m, "", pid_nr_ns(pid, ns));
 	seq_puts(m, " (");
diff --git a/fs/proc/stat.c b/fs/proc/stat.c
index 46b3293015fe..5ae59297591a 100644
--- a/fs/proc/stat.c
+++ b/fs/proc/stat.c
@@ -10,6 +10,7 @@
 #include <linux/seq_file.h>
 #include <linux/slab.h>
 #include <linux/time.h>
+#include <linux/time_namespace.h>
 #include <linux/irqnr.h>
 #include <linux/sched/cputime.h>
 #include <linux/tick.h>
@@ -79,6 +80,20 @@ static u64 get_iowait_time(struct kernel_cpustat *kcs, int cpu)
 
 #endif
 
+static void get_boottime(struct timespec64 *ts)
+{
+	ktime_t boottime;
+
+	/* get kernel internal system boot timestamp */
+	getboottime64(ts);
+
+	/* shift boot timestamp according to the timens offset */
+	boottime = timespec64_to_ktime(*ts);
+	boottime = timens_ktime_to_host(CLOCK_BOOTTIME, boottime);
+
+	*ts = ktime_to_timespec64(boottime);
+}
+
 static void show_irq_gap(struct seq_file *p, unsigned int gap)
 {
 	static const char zeros[] = " 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0";
@@ -117,7 +132,7 @@ static int show_stat(struct seq_file *p, void *v)
 	user = nice = system = idle = iowait =
 		irq = softirq = steal = 0;
 	guest = guest_nice = 0;
-	getboottime64(&boottime);
+	get_boottime(&boottime);
 
 	for_each_possible_cpu(i) {
 		struct kernel_cpustat kcpustat;
-- 
2.20.1

[PATCH v3 3/3] selftests/timens: added selftest for /proc/stat btime

[Michael Weiß]

Test that btime value of /proc/stat is as expected in the time namespace
using a simple parser to get btime from /proc/stat.

Signed-off-by: Michael Weiß <michael.weiss@aisec.fraunhofer.de>
---
 tools/testing/selftests/timens/procfs.c | 58 ++++++++++++++++++++++++-
 1 file changed, 57 insertions(+), 1 deletion(-)

diff --git a/tools/testing/selftests/timens/procfs.c b/tools/testing/selftests/timens/procfs.c
index 7f14f0fdac84..f2519154208a 100644
--- a/tools/testing/selftests/timens/procfs.c
+++ b/tools/testing/selftests/timens/procfs.c
@@ -93,6 +93,33 @@ static int read_proc_uptime(struct timespec *uptime)
 	return 0;
 }
 
+static int read_proc_stat_btime(unsigned long long *boottime_sec)
+{
+	FILE *proc;
+	char line_buf[2048];
+
+	proc = fopen("/proc/stat", "r");
+	if (proc == NULL) {
+		pr_perror("Unable to open /proc/stat");
+		return -1;
+	}
+
+	while (fgets(line_buf, 2048, proc)) {
+		if (sscanf(line_buf, "btime %llu", boottime_sec) != 1)
+			continue;
+		fclose(proc);
+		return 0;
+	}
+	if (errno) {
+		pr_perror("fscanf");
+		fclose(proc);
+		return -errno;
+	}
+	pr_err("failed to parse /proc/stat");
+	fclose(proc);
+	return -1;
+}
+
 static int check_uptime(void)
 {
 	struct timespec uptime_new, uptime_old;
@@ -123,18 +150,47 @@ static int check_uptime(void)
 	return 0;
 }
 
+static int check_stat_btime(void)
+{
+	unsigned long long btime_new, btime_old;
+	unsigned long long btime_expected;
+
+	if (switch_ns(parent_ns))
+		return pr_err("switch_ns(%d)", parent_ns);
+
+	if (read_proc_stat_btime(&btime_old))
+		return 1;
+
+	if (switch_ns(child_ns))
+		return pr_err("switch_ns(%d)", child_ns);
+
+	if (read_proc_stat_btime(&btime_new))
+		return 1;
+
+	btime_expected = btime_old - TEN_DAYS_IN_SEC;
+	if (btime_new != btime_expected) {
+		pr_fail("btime in /proc/stat: old %llu, new %llu [%llu]",
+			btime_old, btime_new, btime_expected);
+		return 1;
+	}
+
+	ksft_test_result_pass("Passed for /proc/stat btime\n");
+	return 0;
+}
+
 int main(int argc, char *argv[])
 {
 	int ret = 0;
 
 	nscheck();
 
-	ksft_set_plan(1);
+	ksft_set_plan(2);
 
 	if (init_namespaces())
 		return 1;
 
 	ret |= check_uptime();
+	ret |= check_stat_btime();
 
 	if (ret)
 		ksft_exit_fail();
-- 
2.20.1

Re: [PATCH v3 2/3] fs/proc: apply the time namespace offset to /proc/stat btime

[Andrei Vagin]

On Sun, Oct 11, 2020 at 04:59:23PM +0200, Michael Weiß wrote:
> @@ -79,6 +80,20 @@ static u64 get_iowait_time(struct kernel_cpustat *kcs, int cpu)
>  
>  #endif
>  
> +static void get_boottime(struct timespec64 *ts)
> +{

> +	ktime_t boottime;
> +
> +	/* get kernel internal system boot timestamp */
> +	getboottime64(ts);
> +
> +	/* shift boot timestamp according to the timens offset */
> +	boottime = timespec64_to_ktime(*ts);
> +	boottime = timens_ktime_to_host(CLOCK_BOOTTIME, boottime);

timens_ktime_to_host is used to convert timens' time to host's time.
Here it looks like we are using it in the opposite direction. I spent
some time to figure out what is going on here. I think it worth to add a
comment here.

> +
> +	*ts = ktime_to_timespec64(boottime);

I don't like all these conversions back and forth. Maybe something like
this will look better:

#ifdef CONFIG_TIME_NS
	if (current->nsproxy->time_ns != init_time_ns) {
		struct timens_offsets *ns_offsets;
		ns_offsets = &current->nsproxy->time_ns->offsets;
		ts = timespec64_sub(ts, timens_offsets->boottime);
	}
#endif


Thanks,
Andrei

Re: [PATCH v3 2/3] fs/proc: apply the time namespace offset to /proc/stat btime

[Michael Weiß]

On 15.10.20 09:53, Andrei Vagin wrote:
> On Sun, Oct 11, 2020 at 04:59:23PM +0200, Michael Weiß wrote:
>> @@ -79,6 +80,20 @@ static u64 get_iowait_time(struct kernel_cpustat *kcs, int cpu)
>>  
>>  #endif
>>  
>> +static void get_boottime(struct timespec64 *ts)
>> +{
> 
>> +	ktime_t boottime;
>> +
>> +	/* get kernel internal system boot timestamp */
>> +	getboottime64(ts);
>> +
>> +	/* shift boot timestamp according to the timens offset */
>> +	boottime = timespec64_to_ktime(*ts);
>> +	boottime = timens_ktime_to_host(CLOCK_BOOTTIME, boottime);
> 
> timens_ktime_to_host is used to convert timens' time to host's time.
> Here it looks like we are using it in the opposite direction. I spent
> some time to figure out what is going on here. I think it worth to add a
> comment here.
> 
>> +
>> +	*ts = ktime_to_timespec64(boottime);
> 
> I don't like all these conversions back and forth. Maybe something like
> this will look better:
> 
> #ifdef CONFIG_TIME_NS
> 	if (current->nsproxy->time_ns != init_time_ns) {
> 		struct timens_offsets *ns_offsets;
> 		ns_offsets = &current->nsproxy->time_ns->offsets;
> 		ts = timespec64_sub(ts, timens_offsets->boottime);
> 	}
> #endif
> 

I agree to that, but maybe we then introduce another helper in
time_namespace.h analogues to timens_add_boottime() and handle
also the ifdef there:

static inline void timens_sub_boottime(struct timespec64 *ts)
{
	struct timens_offsets *ns_offsets = &current->nsproxy->time_ns->offsets;

	*ts = timespec64_sub(*ts, ns_offsets->boottime);
}

using this in proc/stat, it is obvious what is going on and we only need
to add one line of code there. Further, future use-cases which depend on
timens aware boottime would be more strait forward to be implement.

Cheers,
Michael