On Wednesday 27 May 2015 11:17:49 Ksenija Stanojevic wrote:

Replace all instances of time_t with time64_t i.e. change the type used for representing time from 32-bit to 64-bit. All 32-bit kernels to date use a signed 32-bit time_t type, which can only represent time until January 2038. The patch also changes the function get_seconds() that returns a 32-bit integer to ktime_get_seconds() that returns seconds as 64-bit integer and replaces %lx with %lld in one of _debug functions.

Signed-off-by: Ksenija Stanojevic ksenija.stanojevic@gmail.com

The patch looks potentially correct, but at least the description misses one important point: by changing from get_seconds() to ktime_get_seconds(), you use the monotonic time base that starts at boot time.

It will consequently not overrun an unsigned 32-bit number (we can safely assume that the system does not run for over 136 years without a reboot, or 68 years for a signed number), but also needs to ensure that the actual numeric value is never used anywhere as a normal time_t with the 1970 epoch, e.g. in user space or on another machine when exchanged over network packets.

--- a/net/rxrpc/ar-internal.h +++ b/net/rxrpc/ar-internal.h @@ -208,7 +208,7 @@ struct rxrpc_transport { struct rb_root server_conns; /* server connections on this transport */ struct list_head link; /* link in master session list */ struct sk_buff_head error_queue; /* error packets awaiting processing */

• time_t put_time; /* time at which to reap */

• time64_t put_time; /* time at which to reap */ spinlock_t client_lock; /* client connection allocation lock */ rwlock_t conn_lock; /* lock for active/dead connections */ atomic_t usage;

@@ -256,7 +256,7 @@ struct rxrpc_connection { struct rxrpc_crypt csum_iv; /* packet checksum base */ unsigned long events; #define RXRPC_CONN_CHALLENGE 0 /* send challenge packet */

• time_t put_time; /* time at which to reap */

• time64_t put_time; /* time at which to reap */ rwlock_t lock; /* access lock */ spinlock_t state_lock; /* state-change lock */ atomic_t usage;

The two 'put_time' fields get used in two additional files that you did not modify. Since the numeric values are changed now, you have to change them all at once, so they operate on the same time base.

diff --git a/net/rxrpc/ar-key.c b/net/rxrpc/ar-key.c index db0f39f..b2b57cd 100644 --- a/net/rxrpc/ar-key.c +++ b/net/rxrpc/ar-key.c @@ -961,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();

This is a critical function that requires you to research and explain whether your change is safe or not. Have a look at how the expiry value is being used inside of rxrpc_get_server_data_key:

struct rxrpc_key_data_v1 { u16 security_index; u16 ticket_length; u32 expiry; /* time_t */ u32 kvno; u8 session_key[8]; u8 ticket[0]; }; ... struct { u32 kver; struct rxrpc_key_data_v1 v1; } data; ...

data.v1.ticket_length = 0; data.v1.expiry = expiry; ... memcpy(&data.v1.session_key, session_key, sizeof(data.v1.session_key)); ret = key_instantiate_and_link(key, &data, sizeof(data), NULL, NULL);

So the data is being passed in a fixed format as a 32-bit number into another API.

static int rxkad_decrypt_ticket(struct rxrpc_connection *conn, void *ticket, size_t ticket_len, struct rxrpc_crypt *_session_key,

• 		time_t *_expiry,
  

• 		time64_t *_expiry,
  	u32 *_abort_code)
  

{ struct blkcipher_desc desc;

Similarly here:

@@ -827,7 +828,7 @@ static int rxkad_decrypt_ticket(struct rxrpc_connection *conn, struct scatterlist sg[1]; struct in_addr addr; unsigned int life;

• time_t issue, now;

• time64_t issue, now; bool little_endian; int ret; u8 *p, *q, *name, *end;

you change both 'now' and 'issue' to a time64_t.

@@ -922,8 +923,8 @@ static int rxkad_decrypt_ticket(struct rxrpc_connection *conn, issue = be32_to_cpu(stamp); } p += 4;

• now = get_seconds();
• _debug("KIV ISSUE: %lx [%lx]", issue, now);

• now = ktime_get_seconds();
• _debug("KIV ISSUE: %lld [%lld]", issue, now);

For 'now', this is correct, but issue gets assigned above as 'be32_to_cpu(stamp)', so that is a 32-bit value that comes from elsewhere. You need to find out where it comes from, whether it will overflow a 32-bit number in 2038, and whether that type can be changed to 64-bit if it does.

I have my own guesses, but I have not tried to understand that code yet, so I'll let you figure it out yourself.

Arnd