From: Daniel Micay <[email protected]> The WARN_ON() checking for a NULL release pointer was (sensibly) removed in commit ec48c940da6c ("kref: remove WARN_ON for NULL release functions") since it offered no protection at all about calling a NULL release pointer. However, it should instead be a BUG() since continuing with a NULL release pointer will lead to a NULL pointer execution anyway. Systems with an incorrectly set mmap_min_addr and no PXN/SMEP protection would be left open to executing userspace memory.
The kref_put() case is extracted from PaX, and Kees Cook noted it should be extended to the other two cases. Comparison of my build with WARN, with nothing, and with BUG: text data bss dec hex filename 11300251 5586597 13955072 30841920 1d69c40 vmlinux.warn 11298136 5586597 13955072 30839805 1d693fd vmlinux.none 11300062 5586629 13955072 30841763 1d69ba3 vmlinux.bug Signed-off-by: Daniel Micay <[email protected]> [kees: clarify commit log, refreshed diff, moved into if statement] Cc: Andi Kleen <[email protected]> Signed-off-by: Kees Cook <[email protected]> --- include/linux/kref.h | 3 +++ 1 file changed, 3 insertions(+) diff --git a/include/linux/kref.h b/include/linux/kref.h index 29220724bf1c..651a12d2425f 100644 --- a/include/linux/kref.h +++ b/include/linux/kref.h @@ -67,6 +67,7 @@ static inline void kref_get(struct kref *kref) static inline int kref_put(struct kref *kref, void (*release)(struct kref *kref)) { if (refcount_dec_and_test(&kref->refcount)) { + BUG_ON(release == NULL); release(kref); return 1; } @@ -78,6 +79,7 @@ static inline int kref_put_mutex(struct kref *kref, struct mutex *lock) { if (refcount_dec_and_mutex_lock(&kref->refcount, lock)) { + BUG_ON(release == NULL); release(kref); return 1; } @@ -89,6 +91,7 @@ static inline int kref_put_lock(struct kref *kref, spinlock_t *lock) { if (refcount_dec_and_lock(&kref->refcount, lock)) { + BUG_ON(release == NULL); release(kref); return 1; } -- 2.7.4 -- Kees Cook Pixel Security
