[PATCH 0/3] Fix y2038 issues for security/keys subsystem

[PATCH 0/3] Fix y2038 issues for security/keys subsystem

[Baolin Wang]

Since 'time_t', 'timeval' and 'timespec' types are not year 2038 safe on
32 bits system, this patchset tries to fix this issues for security/keys
subsystem and net/rxrpc subsystem which is connected with security/keys
subsystem.

Baolin Wang (3):
  security: keys: Replace time_t/timespec with time64_t
  security: keys: Replace time_t with time64_t for struct
    key_preparsed_payload
  net: rxrpc: Replace time_t type with time64_t type

 include/keys/rxrpc-type.h    |   21 +++++++++++++++++++++
 include/linux/key-type.h     |    2 +-
 include/linux/key.h          |    7 ++++---
 net/rxrpc/ar-internal.h      |    2 +-
 net/rxrpc/key.c              |   22 ++++++++++++++--------
 net/rxrpc/rxkad.c            |   14 +++++++-------
 security/keys/gc.c           |   20 ++++++++++----------
 security/keys/internal.h     |    8 ++++----
 security/keys/key.c          |   27 ++++++++++-----------------
 security/keys/keyring.c      |   18 +++++++++---------
 security/keys/permission.c   |    3 +--
 security/keys/proc.c         |   20 ++++++++++----------
 security/keys/process_keys.c |    2 +-
 13 files changed, 93 insertions(+), 73 deletions(-)

-- 
1.7.9.5

[PATCH 1/3] security: keys: Replace time_t/timespec with time64_t

[Baolin Wang]

The 'struct key' will use 'time_t' which we try to remove in the
kernel, since 'time_t' is not year 2038 safe on 32bit systems.
Also the 'struct keyring_search_context' will use 'timespec' type
to record current time, which is also not year 2038 safe on 32bit
systems.

Thus this patch replaces 'time_t' with 'time64_t' which is year 2038
safe for 'struct key', and replace 'timespec' with 'time64_t' for the
'struct keyring_search_context', since we only look at the the seconds
part of 'timespec' variable. Moreover we also change the codes where
using the 'time_t' and 'timespec', and we can get current time by
ktime_get_real_seconds() instead of current_kernel_time(), and use
'TIME64_MAX' macro to initialize the 'time64_t' type variable.

Especially in proc.c file, we have replaced 'unsigned long' and 'timespec'
type with 'u64' and 'time64_t' type to save the timeout value, which means
user will get one 'u64' type timeout value by issuing proc_keys_show()
function.

Signed-off-by: Baolin Wang <baolin.wang@linaro.org>
---
 include/linux/key.h          |    7 ++++---
 security/keys/gc.c           |   20 ++++++++++----------
 security/keys/internal.h     |    8 ++++----
 security/keys/key.c          |   19 ++++++-------------
 security/keys/keyring.c      |   18 +++++++++---------
 security/keys/permission.c   |    3 +--
 security/keys/proc.c         |   20 ++++++++++----------
 security/keys/process_keys.c |    2 +-
 8 files changed, 45 insertions(+), 52 deletions(-)

diff --git a/include/linux/key.h b/include/linux/key.h
index 0441141..6d10f84 100644
--- a/include/linux/key.h
+++ b/include/linux/key.h
@@ -24,6 +24,7 @@
 #include <linux/atomic.h>
 #include <linux/assoc_array.h>
 #include <linux/refcount.h>
+#include <linux/time64.h>
 
 #ifdef __KERNEL__
 #include <linux/uidgid.h>
@@ -157,10 +158,10 @@ struct key {
 	struct key_user		*user;		/* owner of this key */
 	void			*security;	/* security data for this key */
 	union {
-		time_t		expiry;		/* time at which key expires (or 0) */
-		time_t		revoked_at;	/* time at which key was revoked */
+		time64_t	expiry;		/* time at which key expires (or 0) */
+		time64_t	revoked_at;	/* time at which key was revoked */
 	};
-	time_t			last_used_at;	/* last time used for LRU keyring discard */
+	time64_t		last_used_at;	/* last time used for LRU keyring discard */
 	kuid_t			uid;
 	kgid_t			gid;
 	key_perm_t		perm;		/* access permissions */
diff --git a/security/keys/gc.c b/security/keys/gc.c
index 87cb260..c99700e 100644
--- a/security/keys/gc.c
+++ b/security/keys/gc.c
@@ -32,7 +32,7 @@
 static void key_gc_timer_func(unsigned long);
 static DEFINE_TIMER(key_gc_timer, key_gc_timer_func, 0, 0);
 
-static time_t key_gc_next_run = LONG_MAX;
+static time64_t key_gc_next_run = TIME64_MAX;
 static struct key_type *key_gc_dead_keytype;
 
 static unsigned long key_gc_flags;
@@ -53,12 +53,12 @@ struct key_type key_type_dead = {
  * Schedule a garbage collection run.
  * - time precision isn't particularly important
  */
-void key_schedule_gc(time_t gc_at)
+void key_schedule_gc(time64_t gc_at)
 {
 	unsigned long expires;
-	time_t now = current_kernel_time().tv_sec;
+	time64_t now = ktime_get_real_seconds();
 
-	kenter("%ld", gc_at - now);
+	kenter("%lld", gc_at - now);
 
 	if (gc_at <= now || test_bit(KEY_GC_REAP_KEYTYPE, &key_gc_flags)) {
 		kdebug("IMMEDIATE");
@@ -87,7 +87,7 @@ void key_schedule_gc_links(void)
 static void key_gc_timer_func(unsigned long data)
 {
 	kenter("");
-	key_gc_next_run = LONG_MAX;
+	key_gc_next_run = TIME64_MAX;
 	key_schedule_gc_links();
 }
 
@@ -184,11 +184,11 @@ static void key_garbage_collector(struct work_struct *work)
 
 	struct rb_node *cursor;
 	struct key *key;
-	time_t new_timer, limit;
+	time64_t new_timer, limit;
 
 	kenter("[%lx,%x]", key_gc_flags, gc_state);
 
-	limit = current_kernel_time().tv_sec;
+	limit = ktime_get_real_seconds();
 	if (limit > key_gc_delay)
 		limit -= key_gc_delay;
 	else
@@ -204,7 +204,7 @@ static void key_garbage_collector(struct work_struct *work)
 		gc_state |= KEY_GC_REAPING_DEAD_1;
 	kdebug("new pass %x", gc_state);
 
-	new_timer = LONG_MAX;
+	new_timer = TIME64_MAX;
 
 	/* As only this function is permitted to remove things from the key
 	 * serial tree, if cursor is non-NULL then it will always point to a
@@ -235,7 +235,7 @@ static void key_garbage_collector(struct work_struct *work)
 
 		if (gc_state & KEY_GC_SET_TIMER) {
 			if (key->expiry > limit && key->expiry < new_timer) {
-				kdebug("will expire %x in %ld",
+				kdebug("will expire %x in %lld",
 				       key_serial(key), key->expiry - limit);
 				new_timer = key->expiry;
 			}
@@ -276,7 +276,7 @@ static void key_garbage_collector(struct work_struct *work)
 	 */
 	kdebug("pass complete");
 
-	if (gc_state & KEY_GC_SET_TIMER && new_timer != (time_t)LONG_MAX) {
+	if (gc_state & KEY_GC_SET_TIMER && new_timer != (time64_t)TIME64_MAX) {
 		new_timer += key_gc_delay;
 		key_schedule_gc(new_timer);
 	}
diff --git a/security/keys/internal.h b/security/keys/internal.h
index 91bc621..e23ffbc 100644
--- a/security/keys/internal.h
+++ b/security/keys/internal.h
@@ -130,7 +130,7 @@ struct keyring_search_context {
 	int			skipped_ret;
 	bool			possessed;
 	key_ref_t		result;
-	struct timespec		now;
+	time64_t		now;
 };
 
 extern bool key_default_cmp(const struct key *key,
@@ -169,10 +169,10 @@ extern key_ref_t lookup_user_key(key_serial_t id, unsigned long flags,
 
 extern struct work_struct key_gc_work;
 extern unsigned key_gc_delay;
-extern void keyring_gc(struct key *keyring, time_t limit);
+extern void keyring_gc(struct key *keyring, time64_t limit);
 extern void keyring_restriction_gc(struct key *keyring,
 				   struct key_type *dead_type);
-extern void key_schedule_gc(time_t gc_at);
+extern void key_schedule_gc(time64_t gc_at);
 extern void key_schedule_gc_links(void);
 extern void key_gc_keytype(struct key_type *ktype);
 
@@ -211,7 +211,7 @@ extern struct key *request_key_auth_new(struct key *target,
 /*
  * Determine whether a key is dead.
  */
-static inline bool key_is_dead(const struct key *key, time_t limit)
+static inline bool key_is_dead(const struct key *key, time64_t limit)
 {
 	return
 		key->flags & ((1 << KEY_FLAG_DEAD) |
diff --git a/security/keys/key.c b/security/keys/key.c
index 83da68d..291a67c 100644
--- a/security/keys/key.c
+++ b/security/keys/key.c
@@ -556,7 +556,6 @@ int key_reject_and_link(struct key *key,
 			struct key *authkey)
 {
 	struct assoc_array_edit *edit;
-	struct timespec now;
 	int ret, awaken, link_ret = 0;
 
 	key_check(key);
@@ -582,8 +581,7 @@ int key_reject_and_link(struct key *key,
 		smp_wmb();
 		set_bit(KEY_FLAG_NEGATIVE, &key->flags);
 		set_bit(KEY_FLAG_INSTANTIATED, &key->flags);
-		now = current_kernel_time();
-		key->expiry = now.tv_sec + timeout;
+		key->expiry = ktime_get_real_seconds() + timeout;
 		key_schedule_gc(key->expiry + key_gc_delay);
 
 		if (test_and_clear_bit(KEY_FLAG_USER_CONSTRUCT, &key->flags))
@@ -699,16 +697,13 @@ struct key_type *key_type_lookup(const char *type)
 
 void key_set_timeout(struct key *key, unsigned timeout)
 {
-	struct timespec now;
-	time_t expiry = 0;
+	time64_t expiry = 0;
 
 	/* make the changes with the locks held to prevent races */
 	down_write(&key->sem);
 
-	if (timeout > 0) {
-		now = current_kernel_time();
-		expiry = now.tv_sec + timeout;
-	}
+	if (timeout > 0)
+		expiry = ktime_get_real_seconds() + timeout;
 
 	key->expiry = expiry;
 	key_schedule_gc(key->expiry + key_gc_delay);
@@ -1007,8 +1002,7 @@ int key_update(key_ref_t key_ref, const void *payload, size_t plen)
  */
 void key_revoke(struct key *key)
 {
-	struct timespec now;
-	time_t time;
+	time64_t time;
 
 	key_check(key);
 
@@ -1023,8 +1017,7 @@ void key_revoke(struct key *key)
 		key->type->revoke(key);
 
 	/* set the death time to no more than the expiry time */
-	now = current_kernel_time();
-	time = now.tv_sec;
+	time = ktime_get_real_seconds();
 	if (key->revoked_at == 0 || key->revoked_at > time) {
 		key->revoked_at = time;
 		key_schedule_gc(key->revoked_at + key_gc_delay);
diff --git a/security/keys/keyring.c b/security/keys/keyring.c
index de81793..2d82088 100644
--- a/security/keys/keyring.c
+++ b/security/keys/keyring.c
@@ -576,7 +576,7 @@ static int keyring_search_iterator(const void *object, void *iterator_data)
 			goto skipped;
 		}
 
-		if (key->expiry && ctx->now.tv_sec >= key->expiry) {
+		if (key->expiry && ctx->now >= key->expiry) {
 			if (!(ctx->flags & KEYRING_SEARCH_SKIP_EXPIRED))
 				ctx->result = ERR_PTR(-EKEYEXPIRED);
 			kleave(" = %d [expire]", ctx->skipped_ret);
@@ -837,10 +837,10 @@ static bool search_nested_keyrings(struct key *keyring,
 	key = key_ref_to_ptr(ctx->result);
 	key_check(key);
 	if (!(ctx->flags & KEYRING_SEARCH_NO_UPDATE_TIME)) {
-		key->last_used_at = ctx->now.tv_sec;
-		keyring->last_used_at = ctx->now.tv_sec;
+		key->last_used_at = ctx->now;
+		keyring->last_used_at = ctx->now;
 		while (sp > 0)
-			stack[--sp].keyring->last_used_at = ctx->now.tv_sec;
+			stack[--sp].keyring->last_used_at = ctx->now;
 	}
 	kleave(" = true");
 	return true;
@@ -901,7 +901,7 @@ key_ref_t keyring_search_aux(key_ref_t keyring_ref,
 	}
 
 	rcu_read_lock();
-	ctx->now = current_kernel_time();
+	ctx->now = ktime_get_real_seconds();
 	if (search_nested_keyrings(keyring, ctx))
 		__key_get(key_ref_to_ptr(ctx->result));
 	rcu_read_unlock();
@@ -1147,7 +1147,7 @@ struct key *find_keyring_by_name(const char *name, bool skip_perm_check)
 			 * (ie. it has a zero usage count) */
 			if (!refcount_inc_not_zero(&keyring->usage))
 				continue;
-			keyring->last_used_at = current_kernel_time().tv_sec;
+			keyring->last_used_at = ktime_get_real_seconds();
 			goto out;
 		}
 	}
@@ -1487,7 +1487,7 @@ static void keyring_revoke(struct key *keyring)
 static bool keyring_gc_select_iterator(void *object, void *iterator_data)
 {
 	struct key *key = keyring_ptr_to_key(object);
-	time_t *limit = iterator_data;
+	time64_t *limit = iterator_data;
 
 	if (key_is_dead(key, *limit))
 		return false;
@@ -1498,7 +1498,7 @@ static bool keyring_gc_select_iterator(void *object, void *iterator_data)
 static int keyring_gc_check_iterator(const void *object, void *iterator_data)
 {
 	const struct key *key = keyring_ptr_to_key(object);
-	time_t *limit = iterator_data;
+	time64_t *limit = iterator_data;
 
 	key_check(key);
 	return key_is_dead(key, *limit);
@@ -1510,7 +1510,7 @@ static int keyring_gc_check_iterator(const void *object, void *iterator_data)
  * Not called with any locks held.  The keyring's key struct will not be
  * deallocated under us as only our caller may deallocate it.
  */
-void keyring_gc(struct key *keyring, time_t limit)
+void keyring_gc(struct key *keyring, time64_t limit)
 {
 	int result;
 
diff --git a/security/keys/permission.c b/security/keys/permission.c
index 732cc0b..507b1d41 100644
--- a/security/keys/permission.c
+++ b/security/keys/permission.c
@@ -100,8 +100,7 @@ int key_validate(const struct key *key)
 
 	/* check it hasn't expired */
 	if (key->expiry) {
-		struct timespec now = current_kernel_time();
-		if (now.tv_sec >= key->expiry)
+		if (ktime_get_real_seconds() >= key->expiry)
 			return -EKEYEXPIRED;
 	}
 
diff --git a/security/keys/proc.c b/security/keys/proc.c
index bf08d02..95c8720 100644
--- a/security/keys/proc.c
+++ b/security/keys/proc.c
@@ -178,8 +178,8 @@ static int proc_keys_show(struct seq_file *m, void *v)
 {
 	struct rb_node *_p = v;
 	struct key *key = rb_entry(_p, struct key, serial_node);
-	struct timespec now;
-	unsigned long timo;
+	time64_t now;
+	u64 timo;
 	key_ref_t key_ref, skey_ref;
 	char xbuf[16];
 	int rc;
@@ -216,28 +216,28 @@ static int proc_keys_show(struct seq_file *m, void *v)
 	if (rc < 0)
 		return 0;
 
-	now = current_kernel_time();
+	now = ktime_get_real_seconds();
 
 	rcu_read_lock();
 
 	/* come up with a suitable timeout value */
 	if (key->expiry == 0) {
 		memcpy(xbuf, "perm", 5);
-	} else if (now.tv_sec >= key->expiry) {
+	} else if (now >= key->expiry) {
 		memcpy(xbuf, "expd", 5);
 	} else {
-		timo = key->expiry - now.tv_sec;
+		timo = key->expiry - now;
 
 		if (timo < 60)
-			sprintf(xbuf, "%lus", timo);
+			sprintf(xbuf, "%llus", timo);
 		else if (timo < 60*60)
-			sprintf(xbuf, "%lum", timo / 60);
+			sprintf(xbuf, "%llum", div_u64(timo, 60));
 		else if (timo < 60*60*24)
-			sprintf(xbuf, "%luh", timo / (60*60));
+			sprintf(xbuf, "%lluh", div_u64(timo, 60 * 60));
 		else if (timo < 60*60*24*7)
-			sprintf(xbuf, "%lud", timo / (60*60*24));
+			sprintf(xbuf, "%llud", div_u64(timo, 60 * 60 * 24));
 		else
-			sprintf(xbuf, "%luw", timo / (60*60*24*7));
+			sprintf(xbuf, "%lluw", div_u64(timo, 60 * 60 * 24 * 7));
 	}
 
 #define showflag(KEY, LETTER, FLAG) \
diff --git a/security/keys/process_keys.c b/security/keys/process_keys.c
index 86bced9..c691e09 100644
--- a/security/keys/process_keys.c
+++ b/security/keys/process_keys.c
@@ -736,7 +736,7 @@ key_ref_t lookup_user_key(key_serial_t id, unsigned long lflags,
 	if (ret < 0)
 		goto invalid_key;
 
-	key->last_used_at = current_kernel_time().tv_sec;
+	key->last_used_at = ktime_get_real_seconds();
 
 error:
 	put_cred(ctx.cred);
-- 
1.7.9.5

[PATCH 2/3] security: keys: Replace time_t with time64_t for struct key_preparsed_payload

[Baolin Wang]

The 'struct key_preparsed_payload' will use 'time_t' which we will
try to remove in the kernel, since 'time_t' is not year 2038 safe on
32bits systems.

Thus this patch replaces 'time_t' with 'time64_t' which is year 2038
safe on 32 bits system for 'struct key_preparsed_payload', moreover
we should use the 'TIME64_MAX' macro to initialize the 'time64_t'
type variable.

Signed-off-by: Baolin Wang <baolin.wang@linaro.org>
---
 include/linux/key-type.h |    2 +-
 security/keys/key.c      |    8 ++++----
 2 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/include/linux/key-type.h b/include/linux/key-type.h
index 8496cf6..4beb006 100644
--- a/include/linux/key-type.h
+++ b/include/linux/key-type.h
@@ -44,7 +44,7 @@ struct key_preparsed_payload {
 	const void	*data;		/* Raw data */
 	size_t		datalen;	/* Raw datalen */
 	size_t		quotalen;	/* Quota length for proposed payload */
-	time_t		expiry;		/* Expiry time of key */
+	time64_t	expiry;		/* Expiry time of key */
 };
 
 typedef int (*request_key_actor_t)(struct key_construction *key,
diff --git a/security/keys/key.c b/security/keys/key.c
index 291a67c..d5c8941 100644
--- a/security/keys/key.c
+++ b/security/keys/key.c
@@ -446,7 +446,7 @@ static int __key_instantiate_and_link(struct key *key,
 			if (authkey)
 				key_revoke(authkey);
 
-			if (prep->expiry != TIME_T_MAX) {
+			if (prep->expiry != TIME64_MAX) {
 				key->expiry = prep->expiry;
 				key_schedule_gc(prep->expiry + key_gc_delay);
 			}
@@ -492,7 +492,7 @@ int key_instantiate_and_link(struct key *key,
 	prep.data = data;
 	prep.datalen = datalen;
 	prep.quotalen = key->type->def_datalen;
-	prep.expiry = TIME_T_MAX;
+	prep.expiry = TIME64_MAX;
 	if (key->type->preparse) {
 		ret = key->type->preparse(&prep);
 		if (ret < 0)
@@ -834,7 +834,7 @@ key_ref_t key_create_or_update(key_ref_t keyring_ref,
 	prep.data = payload;
 	prep.datalen = plen;
 	prep.quotalen = index_key.type->def_datalen;
-	prep.expiry = TIME_T_MAX;
+	prep.expiry = TIME64_MAX;
 	if (index_key.type->preparse) {
 		ret = index_key.type->preparse(&prep);
 		if (ret < 0) {
@@ -968,7 +968,7 @@ int key_update(key_ref_t key_ref, const void *payload, size_t plen)
 	prep.data = payload;
 	prep.datalen = plen;
 	prep.quotalen = key->type->def_datalen;
-	prep.expiry = TIME_T_MAX;
+	prep.expiry = TIME64_MAX;
 	if (key->type->preparse) {
 		ret = key->type->preparse(&prep);
 		if (ret < 0)
-- 
1.7.9.5

[PATCH 3/3] net: rxrpc: Replace time_t type with time64_t type

[Baolin Wang]

Since the 'expiry' variable of 'struct key_preparsed_payload' has been
changed to 'time64_t' type, which is year 2038 safe on 32bits system.

In net/rxrpc subsystem, we need convert 'u32' type to 'time64_t' type
when copying ticket expires time to 'prep->expiry', then this patch
introduces two helper functions to help convert 'u32' to 'time64_t'
type.

This patch also uses ktime_get_real_seconds() to get current time instead
of get_seconds() which is not year 2038 safe on 32bits system.

Signed-off-by: Baolin Wang <baolin.wang@linaro.org>
---
 include/keys/rxrpc-type.h |   21 +++++++++++++++++++++
 net/rxrpc/ar-internal.h   |    2 +-
 net/rxrpc/key.c           |   22 ++++++++++++++--------
 net/rxrpc/rxkad.c         |   14 +++++++-------
 4 files changed, 43 insertions(+), 16 deletions(-)

diff --git a/include/keys/rxrpc-type.h b/include/keys/rxrpc-type.h
index 5de0673..76421e2 100644
--- a/include/keys/rxrpc-type.h
+++ b/include/keys/rxrpc-type.h
@@ -127,4 +127,25 @@ struct rxrpc_key_data_v1 {
 #define AFSTOKEN_K5_ADDRESSES_MAX	16	/* max K5 addresses */
 #define AFSTOKEN_K5_AUTHDATA_MAX	16	/* max K5 pieces of auth data */
 
+/*
+ * truncate a time64_t to the range from 1970 to 2106 as
+ * in the network protocol
+ */
+static inline u32 rxrpc_time64_to_u32(time64_t time)
+{
+	if (time < 0)
+		return 0;
+
+	if (time > UINT_MAX)
+		return UINT_MAX;
+
+	return (u32)time;
+}
+
+/* extend u32 back to time64_t using the same 1970-2106 range */
+static inline time64_t rxrpc_u32_to_time64(u32 time)
+{
+	return (time64_t)time;
+}
+
 #endif /* _KEYS_RXRPC_TYPE_H */
diff --git a/net/rxrpc/ar-internal.h b/net/rxrpc/ar-internal.h
index 69b9733..3c11443 100644
--- a/net/rxrpc/ar-internal.h
+++ b/net/rxrpc/ar-internal.h
@@ -894,7 +894,7 @@ void rxrpc_new_incoming_connection(struct rxrpc_sock *,
 
 int rxrpc_request_key(struct rxrpc_sock *, char __user *, int);
 int rxrpc_server_keyring(struct rxrpc_sock *, char __user *, int);
-int rxrpc_get_server_data_key(struct rxrpc_connection *, const void *, time_t,
+int rxrpc_get_server_data_key(struct rxrpc_connection *, const void *, time64_t,
 			      u32);
 
 /*
diff --git a/net/rxrpc/key.c b/net/rxrpc/key.c
index 5436922..e2d3661 100644
--- a/net/rxrpc/key.c
+++ b/net/rxrpc/key.c
@@ -92,6 +92,7 @@ static int rxrpc_preparse_xdr_rxkad(struct key_preparsed_payload *prep,
 				    const __be32 *xdr, unsigned int toklen)
 {
 	struct rxrpc_key_token *token, **pptoken;
+	time64_t expiry;
 	size_t plen;
 	u32 tktlen;
 
@@ -158,8 +159,9 @@ static int rxrpc_preparse_xdr_rxkad(struct key_preparsed_payload *prep,
 	     pptoken = &(*pptoken)->next)
 		continue;
 	*pptoken = token;
-	if (token->kad->expiry < prep->expiry)
-		prep->expiry = token->kad->expiry;
+	expiry = rxrpc_u32_to_time64(token->kad->expiry);
+	if (expiry < prep->expiry)
+		prep->expiry = expiry;
 
 	_leave(" = 0");
 	return 0;
@@ -433,6 +435,7 @@ static int rxrpc_preparse_xdr_rxk5(struct key_preparsed_payload *prep,
 	struct rxrpc_key_token *token, **pptoken;
 	struct rxk5_key *rxk5;
 	const __be32 *end_xdr = xdr + (toklen >> 2);
+	time64_t expiry;
 	int ret;
 
 	_enter(",{%x,%x,%x,%x},%u",
@@ -533,8 +536,9 @@ static int rxrpc_preparse_xdr_rxk5(struct key_preparsed_payload *prep,
 	     pptoken = &(*pptoken)->next)
 		continue;
 	*pptoken = token;
-	if (token->kad->expiry < prep->expiry)
-		prep->expiry = token->kad->expiry;
+	expiry = rxrpc_u32_to_time64(token->kad->expiry);
+	if (expiry < prep->expiry)
+		prep->expiry = expiry;
 
 	_leave(" = 0");
 	return 0;
@@ -691,6 +695,7 @@ static int rxrpc_preparse(struct key_preparsed_payload *prep)
 {
 	const struct rxrpc_key_data_v1 *v1;
 	struct rxrpc_key_token *token, **pp;
+	time64_t expiry;
 	size_t plen;
 	u32 kver;
 	int ret;
@@ -777,8 +782,9 @@ static int rxrpc_preparse(struct key_preparsed_payload *prep)
 	while (*pp)
 		pp = &(*pp)->next;
 	*pp = token;
-	if (token->kad->expiry < prep->expiry)
-		prep->expiry = token->kad->expiry;
+	expiry = rxrpc_u32_to_time64(token->kad->expiry);
+	if (expiry < prep->expiry)
+		prep->expiry = expiry;
 	token = NULL;
 	ret = 0;
 
@@ -955,7 +961,7 @@ int rxrpc_server_keyring(struct rxrpc_sock *rx, char __user *optval,
  */
 int rxrpc_get_server_data_key(struct rxrpc_connection *conn,
 			      const void *session_key,
-			      time_t expiry,
+			      time64_t expiry,
 			      u32 kvno)
 {
 	const struct cred *cred = current_cred();
@@ -982,7 +988,7 @@ int rxrpc_get_server_data_key(struct rxrpc_connection *conn,
 	data.kver = 1;
 	data.v1.security_index = RXRPC_SECURITY_RXKAD;
 	data.v1.ticket_length = 0;
-	data.v1.expiry = expiry;
+	data.v1.expiry = rxrpc_time64_to_u32(expiry);
 	data.v1.kvno = 0;
 
 	memcpy(&data.v1.session_key, session_key, sizeof(data.v1.session_key));
diff --git a/net/rxrpc/rxkad.c b/net/rxrpc/rxkad.c
index 46d1a1f..34c86d2 100644
--- a/net/rxrpc/rxkad.c
+++ b/net/rxrpc/rxkad.c
@@ -854,7 +854,7 @@ static int rxkad_decrypt_ticket(struct rxrpc_connection *conn,
 				struct sk_buff *skb,
 				void *ticket, size_t ticket_len,
 				struct rxrpc_crypt *_session_key,
-				time_t *_expiry,
+				time64_t *_expiry,
 				u32 *_abort_code)
 {
 	struct skcipher_request *req;
@@ -864,7 +864,7 @@ static int rxkad_decrypt_ticket(struct rxrpc_connection *conn,
 	struct in_addr addr;
 	unsigned int life;
 	const char *eproto;
-	time_t issue, now;
+	time64_t issue, now;
 	bool little_endian;
 	int ret;
 	u32 abort_code;
@@ -960,15 +960,15 @@ static int rxkad_decrypt_ticket(struct rxrpc_connection *conn,
 	if (little_endian) {
 		__le32 stamp;
 		memcpy(&stamp, p, 4);
-		issue = le32_to_cpu(stamp);
+		issue = rxrpc_u32_to_time64(le32_to_cpu(stamp));
 	} else {
 		__be32 stamp;
 		memcpy(&stamp, p, 4);
-		issue = be32_to_cpu(stamp);
+		issue = rxrpc_u32_to_time64(be32_to_cpu(stamp));
 	}
 	p += 4;
-	now = get_seconds();
-	_debug("KIV ISSUE: %lx [%lx]", issue, now);
+	now = ktime_get_real_seconds();
+	_debug("KIV ISSUE: %llx [%llx]", issue, now);
 
 	/* check the ticket is in date */
 	if (issue > now) {
@@ -1053,7 +1053,7 @@ static int rxkad_verify_response(struct rxrpc_connection *conn,
 	struct rxrpc_skb_priv *sp = rxrpc_skb(skb);
 	struct rxrpc_crypt session_key;
 	const char *eproto;
-	time_t expiry;
+	time64_t expiry;
 	void *ticket;
 	u32 abort_code, version, kvno, ticket_len, level;
 	__be32 csum;
-- 
1.7.9.5

Re: [PATCH 0/3] Fix y2038 issues for security/keys subsystem

[David Howells]

The rxrpc patch isn't part of the security/keys subsystem.  I'll push it
to the network tree.  The other two I'll push to James.

David

Re: [PATCH 0/3] Fix y2038 issues for security/keys subsystem

[Arnd Bergmann]

On Wed, Aug 9, 2017 at 4:51 AM, Baolin Wang <baolin.wang@linaro.org> wrote:
> Since 'time_t', 'timeval' and 'timespec' types are not year 2038 safe on
> 32 bits system, this patchset tries to fix this issues for security/keys
> subsystem and net/rxrpc subsystem which is connected with security/keys
> subsystem.
>
> Baolin Wang (3):
>   security: keys: Replace time_t/timespec with time64_t
>   security: keys: Replace time_t with time64_t for struct
>     key_preparsed_payload
>   net: rxrpc: Replace time_t type with time64_t type

Hi David,

I did a private review before Baolin posted these patches, this version look
correct to me, though I would like to see some clarification from you for the
rxrpc portion, I'll reply there separately.

All three patches

Reviewed-by: Arnd Bergmann <arnd@arndb.de>

Re: [PATCH 3/3] net: rxrpc: Replace time_t type with time64_t type

[Arnd Bergmann]

On Wed, Aug 9, 2017 at 4:51 AM, Baolin Wang <baolin.wang@linaro.org> wrote:

> diff --git a/include/keys/rxrpc-type.h b/include/keys/rxrpc-type.h
> index 5de0673..76421e2 100644
> --- a/include/keys/rxrpc-type.h
> +++ b/include/keys/rxrpc-type.h
> @@ -127,4 +127,25 @@ struct rxrpc_key_data_v1 {
>  #define AFSTOKEN_K5_ADDRESSES_MAX      16      /* max K5 addresses */
>  #define AFSTOKEN_K5_AUTHDATA_MAX       16      /* max K5 pieces of auth data */
>
> +/*
> + * truncate a time64_t to the range from 1970 to 2106 as
> + * in the network protocol
> + */
> +static inline u32 rxrpc_time64_to_u32(time64_t time)
> +{
> +       if (time < 0)
> +               return 0;
> +
> +       if (time > UINT_MAX)
> +               return UINT_MAX;
> +
> +       return (u32)time;
> +}
>+
> +/* extend u32 back to time64_t using the same 1970-2106 range */
> +static inline time64_t rxrpc_u32_to_time64(u32 time)
> +{
> +       return (time64_t)time;
> +}

When reviewing this, I could not find any clear definition on whether
the preparse functions should treat this as a signed or unsigned
32-bit number. The function as defined here documents as an unsigned
as that is more useful (it pushes out the last day this works from year
2038 to 2106) and matches the existing behavior that we got on
64-bit architectures (intentionally or not).

> @@ -433,6 +435,7 @@ static int rxrpc_preparse_xdr_rxk5(struct key_preparsed_payload *prep,
>         struct rxrpc_key_token *token, **pptoken;
>         struct rxk5_key *rxk5;
>         const __be32 *end_xdr = xdr + (toklen >> 2);
> +       time64_t expiry;
>         int ret;
>
>         _enter(",{%x,%x,%x,%x},%u",
> @@ -533,8 +536,9 @@ static int rxrpc_preparse_xdr_rxk5(struct key_preparsed_payload *prep,
>              pptoken = &(*pptoken)->next)
>                 continue;
>         *pptoken = token;
> -       if (token->kad->expiry < prep->expiry)
> -               prep->expiry = token->kad->expiry;
> +       expiry = rxrpc_u32_to_time64(token->kad->expiry);
> +       if (expiry < prep->expiry)
> +               prep->expiry = expiry;
>
>         _leave(" = 0");
>         return 0;

I'm still slightly puzzled by what this function does: it does have
four timestamps
(authtime, starttime, endtime, renew_till) that are all transferred as
64-bit values
and won't expire, but then it also uses the 32-bit expiry field in
rxrpc_key_token->kad->expiry instead of the 64-bit rxrpc_key_token->k5
fields.

This appears to overlay the first 32 bits of the rxrpc_key_token->k5->starttime
field, which is also a time value on little-endian architectures by chance,
but I would assume that it's always in the past, meaning the keys would
already be expired. Any idea what is the intended behavior here?

       Arnd

Re: [PATCH 3/3] net: rxrpc: Replace time_t type with time64_t type

[David Howells]

Arnd Bergmann <arnd@arndb.de> wrote:

> > @@ -533,8 +536,9 @@ static int rxrpc_preparse_xdr_rxk5(struct key_preparsed_payload *prep,
> >              pptoken = &(*pptoken)->next)
> >                 continue;
> >         *pptoken = token;
> > -       if (token->kad->expiry < prep->expiry)
> > -               prep->expiry = token->kad->expiry;
> ...
>
> I'm still slightly puzzled by what this function does: it does have four
> timestamps (authtime, starttime, endtime, renew_till) that are all
> transferred as 64-bit values and won't expire, but then it also uses the
> 32-bit expiry field in rxrpc_key_token->kad->expiry instead of the 64-bit
> rxrpc_key_token->k5 fields.

Good catch.  This is a cut'n'paste error.  It should be using
token->k5->expiry here not token->kad->expiry.

> This appears to overlay the first 32 bits of the
> rxrpc_key_token->k5->starttime field, which is also a time value on
> little-endian architectures by chance, but I would assume that it's always
> in the past, meaning the keys would already be expired.

Yeah - I'm not sure why it works.

David

Re: [PATCH 3/3] net: rxrpc: Replace time_t type with time64_t type

[Arnd Bergmann]

On Wed, Aug 9, 2017 at 11:33 AM, David Howells <dhowells@redhat.com> wrote:
> Arnd Bergmann <arnd@arndb.de> wrote:
>
>> > @@ -533,8 +536,9 @@ static int rxrpc_preparse_xdr_rxk5(struct key_preparsed_payload *prep,
>> >              pptoken = &(*pptoken)->next)
>> >                 continue;
>> >         *pptoken = token;
>> > -       if (token->kad->expiry < prep->expiry)
>> > -               prep->expiry = token->kad->expiry;
>> ...
>>
>> I'm still slightly puzzled by what this function does: it does have four
>> timestamps (authtime, starttime, endtime, renew_till) that are all
>> transferred as 64-bit values and won't expire, but then it also uses the
>> 32-bit expiry field in rxrpc_key_token->kad->expiry instead of the 64-bit
>> rxrpc_key_token->k5 fields.
>
> Good catch.  This is a cut'n'paste error.  It should be using
> token->k5->expiry here not token->kad->expiry.

Do you know which format is used in practice? Are both kad and k5 common
among rxrpc users?

      Arnd

Re: [PATCH 3/3] net: rxrpc: Replace time_t type with time64_t type

[David Howells]

Arnd Bergmann <arnd@arndb.de> wrote:

> Do you know which format is used in practice? Are both kad and k5 common
> among rxrpc users?

The aklog program I'm using uses the non-XDR interface to push a Kerberos 5
ticket to the kernel, so it doesn't actually invoke rxrpc_preparse_xdr() from
rxrpc_preparse().

David

Re: [PATCH 3/3] net: rxrpc: Replace time_t type with time64_t type

[Arnd Bergmann]

On Wed, Aug 9, 2017 at 3:26 PM, David Howells <dhowells@redhat.com> wrote:
> Arnd Bergmann <arnd@arndb.de> wrote:
>
>> Do you know which format is used in practice? Are both kad and k5 common
>> among rxrpc users?
>
> The aklog program I'm using uses the non-XDR interface to push a Kerberos 5
> ticket to the kernel, so it doesn't actually invoke rxrpc_preparse_xdr() from
> rxrpc_preparse().

Ah, I'm slowly starting to understand how this fits together. So you can add
a key either through key_add() from local user space, or through an rxrpc
socket.

>From what I can tell, the program you have at
http://people.redhat.com/~dhowells/rxrpc/klog.c will keep working beyond
2038 but not beyond 2106 on all 64-bit architectures and on those
32-bit systems that have a libc with 64-bit time_t. It could be modified
to use the xdr_rxk5 key format, which would make it use 64-bit time
values (and require the kernel fix mentioned above).

In contrast, the rxrpc socket interface would need a major rework to
support 64-bit expiration times. It receives a kerberos ticket with a
32-bit issue time
that gets used to calculate the expiry time in rxkad_decrypt_ticket, and from
there we pass it through a  rxrpc_key_data_v1 into key_instantiate_and_link(),
which calls rxrpc_preparse() and that just takes the expiry out again and sticks
it into another 32-bit field in struct rxkad_key, from where it
finally gets copied
into the (now 64-bit) key_preparsed_payload->expiry field.

Does my understanding match what you intended for the interfaces? Is there
a need to extend the rxrpc socket interface to support xdr_rxk5 keys as well?

         Arnd

Re: [PATCH 3/3] net: rxrpc: Replace time_t type with time64_t type

[David Howells]

Arnd Bergmann <arnd@arndb.de> wrote:

> Ah, I'm slowly starting to understand how this fits together. So you can add
> a key either through key_add() from local user space, or through an rxrpc
> socket.

No, you can't add keys through an rxrpc socket.

There are three 'classes' of key:

 (1) Client keys (type rxrpc).  These must be added by add_key() by userspace
     (but could also be acquired by upcalling to /sbin/request-key) and then
     the kernel calls request_key() to locate them on entry through either a
     kafs inode/file operation or through sendmsg() to an AF_RXRPC socket.

 (2) Server keys (type rxrpc_s).  These are created by userspace and are
     presented to an AF_RXRPC server socket by calling setsockopt().  The
     server uses these to validate/decrypt the token passed by a RESPONSE
     packet.

 (3) Service connection keys (type rxrpc).  These are created internally by
     AF_RXRPC after a successful challenge/response negotiation to hold the
     security details so that we have a struct key to pass around that
     corresponds to the key in (1).

David

Re: [PATCH 0/3] Fix y2038 issues for security/keys subsystem

[Baolin Wang]

On 9 August 2017 at 16:28, David Howells <dhowells@redhat.com> wrote:
> The rxrpc patch isn't part of the security/keys subsystem.  I'll push it
> to the network tree.  The other two I'll push to James.

Thanks David.

-- 
Baolin.wang
Best Regards

Re: [PATCH 0/3] Fix y2038 issues for security/keys subsystem

[Baolin Wang]

Hi Arnd,

On 9 August 2017 at 16:44, Arnd Bergmann <arnd@arndb.de> wrote:
> On Wed, Aug 9, 2017 at 4:51 AM, Baolin Wang <baolin.wang@linaro.org> wrote:
>> Since 'time_t', 'timeval' and 'timespec' types are not year 2038 safe on
>> 32 bits system, this patchset tries to fix this issues for security/keys
>> subsystem and net/rxrpc subsystem which is connected with security/keys
>> subsystem.
>>
>> Baolin Wang (3):
>>   security: keys: Replace time_t/timespec with time64_t
>>   security: keys: Replace time_t with time64_t for struct
>>     key_preparsed_payload
>>   net: rxrpc: Replace time_t type with time64_t type
>
> Hi David,
>
> I did a private review before Baolin posted these patches, this version look
> correct to me, though I would like to see some clarification from you for the
> rxrpc portion, I'll reply there separately.
>
> All three patches
>
> Reviewed-by: Arnd Bergmann <arnd@arndb.de>

Thanks for your reviewed tag.

-- 
Baolin.wang
Best Regards

Re: [PATCH 0/3] Fix y2038 issues for security/keys subsystem

[Baolin Wang]

Hi David and James,

On 9 August 2017 at 16:28, David Howells <dhowells@redhat.com> wrote:
> The rxrpc patch isn't part of the security/keys subsystem.  I'll push it
> to the network tree.  The other two I'll push to James.

Could you apply this patch serials if there are no other comments? Thanks.

-- 
Baolin.wang
Best Regards

Re: [PATCH 0/3] Fix y2038 issues for security/keys subsystem

[Baolin Wang]

Hi James,

On 21 August 2017 at 20:12, Baolin Wang <baolin.wang@linaro.org> wrote:
> Hi David and James,
>
> On 9 August 2017 at 16:28, David Howells <dhowells@redhat.com> wrote:
>> The rxrpc patch isn't part of the security/keys subsystem.  I'll push it
>> to the network tree.  The other two I'll push to James.
>
> Could you apply this patch serials if there are no other comments? Thanks.

Could you apply patch1 and patch2? I have seen patch3 was merged into
4.14 by David. Thanks.

-- 
Baolin.wang
Best Regards