History

mm: add kvfree_sensitive() for freeing sensitive data objects

For kvmalloc'ed data object that contains sensitive information like cryptographic keys, we need to make sure that the buffer is always cleared before freeing it. Using memset() alone for buffer clearing may not provide certainty as the compiler may compile it away. To be sure, the special memzero_explicit() has to be used. This patch introduces a new kvfree_sensitive() for freeing those sensitive data objects allocated by kvmalloc(). The relevant places where kvfree_sensitive() can be used are modified to use it.

Waiman Long committed 6 years ago

Commit d4eaa281 parent 090e77e4 files · +24 −22Browse files

Unified · Split

4 files
  1. include/linux/mm.h +1 −0
  2. mm/util.c +18 −0
  3. security/keys/internal.h +0 −11
  4. security/keys/keyctl.c +5 −11
include/linux/mm.h+1−0
@@ -776,6 +776,7 @@ static inline void *kvcalloc(size_t n, size_t size, gfp_t flags) · 775 unmodified lines
776776 }
777777
778778 extern void kvfree(const void *addr);
779+extern void kvfree_sensitive(const void *addr, size_t len);
779780
780781 /*
781782 * Mapcount of compound page as a whole, does not include mapped sub-pages.
mm/util.c+18−0
@@ -604,6 +604,24 @@ void kvfree(const void *addr) · 603 unmodified lines
604604 }
605605 EXPORT_SYMBOL(kvfree);
606606
607+/**
608+ * kvfree_sensitive - Free a data object containing sensitive information.
609+ * @addr: address of the data object to be freed.
610+ * @len: length of the data object.
611+ *
612+ * Use the special memzero_explicit() function to clear the content of a
613+ * kvmalloc'ed object containing sensitive data to make sure that the
614+ * compiler won't optimize out the data clearing.
615+ */
616+void kvfree_sensitive(const void *addr, size_t len)
617+{
618+ if (likely(!ZERO_OR_NULL_PTR(addr))) {
619+ memzero_explicit((void *)addr, len);
620+ kvfree(addr);
621+ }
622+}
623+EXPORT_SYMBOL(kvfree_sensitive);
624+
607625 static inline void *__page_rmapping(struct page *page)
608626 {
609627 unsigned long mapping;
security/keys/internal.h+0−11
@@ -350,15 +350,4 @@ static inline void key_check(const struct key *key) · 349 unmodified lines
350350 #define key_check(key) do {} while(0)
351351
352352 #endif
353−
354−/*
355− * Helper function to clear and free a kvmalloc'ed memory object.
356− */
357−static inline void __kvzfree(const void *addr, size_t len)
358−{
359− if (addr) {
360− memset((void *)addr, 0, len);
361− kvfree(addr);
362− }
363−}
364353 #endif /* _INTERNAL_H */
security/keys/keyctl.c+5−11
@@ -142,10 +142,7 @@ SYSCALL_DEFINE5(add_key, const char __user *, _type, · 141 unmodified lines
142142
143143 key_ref_put(keyring_ref);
144144 error3:
145− if (payload) {
146− memzero_explicit(payload, plen);
147− kvfree(payload);
148− }
145+ kvfree_sensitive(payload, plen);
149146 error2:
150147 kfree(description);
151148 error:
@@ -360,7 +357,7 @@ long keyctl_update_key(key_serial_t id, · 208 unmodified lines
360357
361358 key_ref_put(key_ref);
362359 error2:
363− __kvzfree(payload, plen);
360+ kvfree_sensitive(payload, plen);
364361 error:
365362 return ret;
366363 }
@@ -914,7 +911,7 @@ can_read_key: · 547 unmodified lines
914911 */
915912 if (ret > key_data_len) {
916913 if (unlikely(key_data))
917− __kvzfree(key_data, key_data_len);
914+ kvfree_sensitive(key_data, key_data_len);
918915 key_data_len = ret;
919916 continue; /* Allocate buffer */
920917 }
@@ -923,7 +920,7 @@ can_read_key: · 2 unmodified lines
923920 ret = -EFAULT;
924921 break;
925922 }
926− __kvzfree(key_data, key_data_len);
923+ kvfree_sensitive(key_data, key_data_len);
927924
928925 key_put_out:
929926 key_put(key);
@@ -1225,10 +1222,7 @@ long keyctl_instantiate_key_common(key_serial_t id, · 295 unmodified lines
12251222 keyctl_change_reqkey_auth(NULL);
12261223
12271224 error2:
1228− if (payload) {
1229− memzero_explicit(payload, plen);
1230− kvfree(payload);
1231− }
1225+ kvfree_sensitive(payload, plen);
12321226 error:
12331227 return ret;
12341228 }