From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753278AbdF0TAJ (ORCPT ); Tue, 27 Jun 2017 15:00:09 -0400 Received: from mail-pg0-f42.google.com ([74.125.83.42]:32833 "EHLO mail-pg0-f42.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752311AbdF0TAE (ORCPT ); Tue, 27 Jun 2017 15:00:04 -0400 Date: Tue, 27 Jun 2017 12:00:02 -0700 From: Kees Cook To: Greg Kroah-Hartman Cc: Ingo Molnar , Peter Zijlstra , "Jason A. Donenfeld" , Thomas Hellstrom , Andi Kleen , Daniel Micay , linux-kernel@vger.kernel.org Subject: [PATCH v2] kref: Avoid null pointer dereference after WARN Message-ID: <20170627190001.GA7811@beast> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org From: Daniel Micay 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 [kees: clarify commit log, refreshed diff, moved into if statement] Cc: Andi Kleen Signed-off-by: Kees Cook --- 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