Commit c08ef808ef24df32e25fbd949fe5310172f3c408

Authored by David Howells
Committed by James Morris
1 parent 5c84342a3e

KEYS: Fix garbage collector

Fix a number of problems with the new key garbage collector:

 (1) A rogue semicolon in keyring_gc() was causing the initial count of dead
     keys to be miscalculated.

 (2) A missing return in keyring_gc() meant that under certain circumstances,
     the keyring semaphore would be unlocked twice.

 (3) The key serial tree iterator (key_garbage_collector()) part of the garbage
     collector has been modified to:

     (a) Complete each scan of the keyrings before setting the new timer.

     (b) Only set the new timer for keys that have yet to expire.  This means
         that the new timer is now calculated correctly, and the gc doesn't
         get into a loop continually scanning for keys that have expired, and
         preventing other things from happening, like RCU cleaning up the old
         keyring contents.

     (c) Perform an extra scan if any keys were garbage collected in this one
     	 as a key might become garbage during a scan, and (b) could mean we
     	 don't set the timer again.

 (4) Made key_schedule_gc() take the time at which to do a collection run,
     rather than the time at which the key expires.  This means the collection
     of dead keys (key type unregistered) can happen immediately.

Signed-off-by: David Howells <dhowells@redhat.com>
Signed-off-by: James Morris <jmorris@namei.org>

Showing 4 changed files with 73 additions and 35 deletions Side-by-side Diff

... ... @@ -26,8 +26,10 @@
26 26 static DEFINE_TIMER(key_gc_timer, key_gc_timer_func, 0, 0);
27 27 static DECLARE_WORK(key_gc_work, key_garbage_collector);
28 28 static key_serial_t key_gc_cursor; /* the last key the gc considered */
  29 +static bool key_gc_again;
29 30 static unsigned long key_gc_executing;
30 31 static time_t key_gc_next_run = LONG_MAX;
  32 +static time_t key_gc_new_timer;
31 33  
32 34 /*
33 35 * Schedule a garbage collection run
... ... @@ -40,9 +42,7 @@
40 42  
41 43 kenter("%ld", gc_at - now);
42 44  
43   - gc_at += key_gc_delay;
44   -
45   - if (now >= gc_at) {
  45 + if (gc_at <= now) {
46 46 schedule_work(&key_gc_work);
47 47 } else if (gc_at < key_gc_next_run) {
48 48 expires = jiffies + (gc_at - now) * HZ;
49 49  
50 50  
51 51  
... ... @@ -112,16 +112,18 @@
112 112 struct rb_node *rb;
113 113 key_serial_t cursor;
114 114 struct key *key, *xkey;
115   - time_t new_timer = LONG_MAX, limit;
  115 + time_t new_timer = LONG_MAX, limit, now;
116 116  
117   - kenter("");
  117 + now = current_kernel_time().tv_sec;
  118 + kenter("[%x,%ld]", key_gc_cursor, key_gc_new_timer - now);
118 119  
119 120 if (test_and_set_bit(0, &key_gc_executing)) {
120   - key_schedule_gc(current_kernel_time().tv_sec);
  121 + key_schedule_gc(current_kernel_time().tv_sec + 1);
  122 + kleave(" [busy; deferring]");
121 123 return;
122 124 }
123 125  
124   - limit = current_kernel_time().tv_sec;
  126 + limit = now;
125 127 if (limit > key_gc_delay)
126 128 limit -= key_gc_delay;
127 129 else
128 130  
... ... @@ -129,12 +131,19 @@
129 131  
130 132 spin_lock(&key_serial_lock);
131 133  
132   - if (RB_EMPTY_ROOT(&key_serial_tree))
133   - goto reached_the_end;
  134 + if (unlikely(RB_EMPTY_ROOT(&key_serial_tree))) {
  135 + spin_unlock(&key_serial_lock);
  136 + clear_bit(0, &key_gc_executing);
  137 + return;
  138 + }
134 139  
135 140 cursor = key_gc_cursor;
136 141 if (cursor < 0)
137 142 cursor = 0;
  143 + if (cursor > 0)
  144 + new_timer = key_gc_new_timer;
  145 + else
  146 + key_gc_again = false;
138 147  
139 148 /* find the first key above the cursor */
140 149 key = NULL;
141 150  
142 151  
143 152  
144 153  
145 154  
146 155  
147 156  
148 157  
... ... @@ -160,36 +169,51 @@
160 169  
161 170 /* trawl through the keys looking for keyrings */
162 171 for (;;) {
163   - if (key->expiry > 0 && key->expiry < new_timer)
  172 + if (key->expiry > now && key->expiry < new_timer) {
  173 + kdebug("will expire %x in %ld",
  174 + key_serial(key), key->expiry - now);
164 175 new_timer = key->expiry;
  176 + }
165 177  
166 178 if (key->type == &key_type_keyring &&
167   - key_gc_keyring(key, limit)) {
168   - /* the gc ate our lock */
169   - schedule_work(&key_gc_work);
170   - goto no_unlock;
171   - }
  179 + key_gc_keyring(key, limit))
  180 + /* the gc had to release our lock so that the keyring
  181 + * could be modified, so we have to get it again */
  182 + goto gc_released_our_lock;
172 183  
173 184 rb = rb_next(&key->serial_node);
174   - if (!rb) {
175   - key_gc_cursor = 0;
176   - break;
177   - }
  185 + if (!rb)
  186 + goto reached_the_end;
178 187 key = rb_entry(rb, struct key, serial_node);
179 188 }
180 189  
181   -out:
182   - spin_unlock(&key_serial_lock);
183   -no_unlock:
  190 +gc_released_our_lock:
  191 + kdebug("gc_released_our_lock");
  192 + key_gc_new_timer = new_timer;
  193 + key_gc_again = true;
184 194 clear_bit(0, &key_gc_executing);
185   - if (new_timer < LONG_MAX)
186   - key_schedule_gc(new_timer);
187   -
188   - kleave("");
  195 + schedule_work(&key_gc_work);
  196 + kleave(" [continue]");
189 197 return;
190 198  
  199 + /* when we reach the end of the run, we set the timer for the next one */
191 200 reached_the_end:
  201 + kdebug("reached_the_end");
  202 + spin_unlock(&key_serial_lock);
  203 + key_gc_new_timer = new_timer;
192 204 key_gc_cursor = 0;
193   - goto out;
  205 + clear_bit(0, &key_gc_executing);
  206 +
  207 + if (key_gc_again) {
  208 + /* there may have been a key that expired whilst we were
  209 + * scanning, so if we discarded any links we should do another
  210 + * scan */
  211 + new_timer = now + 1;
  212 + key_schedule_gc(new_timer);
  213 + } else if (new_timer < LONG_MAX) {
  214 + new_timer += key_gc_delay;
  215 + key_schedule_gc(new_timer);
  216 + }
  217 + kleave(" [end]");
194 218 }
... ... @@ -500,7 +500,7 @@
500 500 set_bit(KEY_FLAG_INSTANTIATED, &key->flags);
501 501 now = current_kernel_time();
502 502 key->expiry = now.tv_sec + timeout;
503   - key_schedule_gc(key->expiry);
  503 + key_schedule_gc(key->expiry + key_gc_delay);
504 504  
505 505 if (test_and_clear_bit(KEY_FLAG_USER_CONSTRUCT, &key->flags))
506 506 awaken = 1;
... ... @@ -909,7 +909,7 @@
909 909 time = now.tv_sec;
910 910 if (key->revoked_at == 0 || key->revoked_at > time) {
911 911 key->revoked_at = time;
912   - key_schedule_gc(key->revoked_at);
  912 + key_schedule_gc(key->revoked_at + key_gc_delay);
913 913 }
914 914  
915 915 up_write(&key->sem);
security/keys/keyctl.c
... ... @@ -1115,7 +1115,7 @@
1115 1115 }
1116 1116  
1117 1117 key->expiry = expiry;
1118   - key_schedule_gc(key->expiry);
  1118 + key_schedule_gc(key->expiry + key_gc_delay);
1119 1119  
1120 1120 up_write(&key->sem);
1121 1121 key_put(key);
security/keys/keyring.c
... ... @@ -1019,18 +1019,18 @@
1019 1019 struct key *key;
1020 1020 int loop, keep, max;
1021 1021  
1022   - kenter("%x", key_serial(keyring));
  1022 + kenter("{%x,%s}", key_serial(keyring), keyring->description);
1023 1023  
1024 1024 down_write(&keyring->sem);
1025 1025  
1026 1026 klist = keyring->payload.subscriptions;
1027 1027 if (!klist)
1028   - goto just_return;
  1028 + goto no_klist;
1029 1029  
1030 1030 /* work out how many subscriptions we're keeping */
1031 1031 keep = 0;
1032 1032 for (loop = klist->nkeys - 1; loop >= 0; loop--)
1033   - if (!key_is_dead(klist->keys[loop], limit));
  1033 + if (!key_is_dead(klist->keys[loop], limit))
1034 1034 keep++;
1035 1035  
1036 1036 if (keep == klist->nkeys)
... ... @@ -1041,7 +1041,7 @@
1041 1041 new = kmalloc(sizeof(struct keyring_list) + max * sizeof(struct key *),
1042 1042 GFP_KERNEL);
1043 1043 if (!new)
1044   - goto just_return;
  1044 + goto nomem;
1045 1045 new->maxkeys = max;
1046 1046 new->nkeys = 0;
1047 1047 new->delkey = 0;
1048 1048  
... ... @@ -1081,8 +1081,22 @@
1081 1081 discard_new:
1082 1082 new->nkeys = keep;
1083 1083 keyring_clear_rcu_disposal(&new->rcu);
  1084 + up_write(&keyring->sem);
  1085 + kleave(" [discard]");
  1086 + return;
  1087 +
1084 1088 just_return:
1085 1089 up_write(&keyring->sem);
1086   - kleave(" [no]");
  1090 + kleave(" [no dead]");
  1091 + return;
  1092 +
  1093 +no_klist:
  1094 + up_write(&keyring->sem);
  1095 + kleave(" [no_klist]");
  1096 + return;
  1097 +
  1098 +nomem:
  1099 + up_write(&keyring->sem);
  1100 + kleave(" [oom]");
1087 1101 }