Messages in this thread Patch in this message | | | Date | Tue, 27 Jun 2017 12:00:02 -0700 | From | Kees Cook <> | Subject | [PATCH v2] kref: Avoid null pointer dereference after WARN |
| |
From: Daniel Micay <danielmicay@gmail.com>
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 <danielmicay@gmail.com> [kees: clarify commit log, refreshed diff, moved into if statement] Cc: Andi Kleen <ak@linux.intel.com> Signed-off-by: Kees Cook <keescook@chromium.org> --- 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
| |