kernel-hardening.lists.openwall.com archive mirror
 help / color / mirror / Atom feed
From: Laura Abbott <labbott@fedoraproject.org>
To: Christoph Lameter <cl@linux.com>,
	Pekka Enberg <penberg@kernel.org>,
	David Rientjes <rientjes@google.com>,
	Joonsoo Kim <js1304@gmail.com>,
	Andrew Morton <akpm@linux-foundation.org>
Cc: Laura Abbott <labbott@fedoraproject.org>,
	linux-mm@kvack.org, linux-kernel@vger.kernel.org,
	kernel-hardening@lists.openwall.com,
	Kees Cook <keescook@chromium.org>
Subject: [kernel-hardening] [PATCHv2 2/4] slub: Fix/clean free_debug_processing return paths
Date: Mon, 15 Feb 2016 10:44:22 -0800	[thread overview]
Message-ID: <1455561864-4217-3-git-send-email-labbott@fedoraproject.org> (raw)
In-Reply-To: <1455561864-4217-1-git-send-email-labbott@fedoraproject.org>


Since 19c7ff9ecd89 ("slub: Take node lock during object free checks")
check_object has been incorrectly returning success as it follows
the out label which just returns the node. Thanks to refactoring,
the out and fail paths are now basically the same. Combine the two
into one and just use a single label.

Credit to Mathias Krause for the original work which inspired this series

Signed-off-by: Laura Abbott <labbott@fedoraproject.org>
---
If there is interest, I can split this off as a separate patch for stable
---
 mm/slub.c | 21 ++++++++++-----------
 1 file changed, 10 insertions(+), 11 deletions(-)

diff --git a/mm/slub.c b/mm/slub.c
index 2d5a774..189c330 100644
--- a/mm/slub.c
+++ b/mm/slub.c
@@ -1077,24 +1077,25 @@ static noinline int free_debug_processing(
 	void *object = head;
 	int cnt = 0;
 	unsigned long uninitialized_var(flags);
+	int ret = 0;
 
 	spin_lock_irqsave(&n->list_lock, flags);
 	slab_lock(page);
 
 	if (!check_slab(s, page))
-		goto fail;
+		goto out;
 
 next_object:
 	cnt++;
 
 	if (!check_valid_pointer(s, page, object)) {
 		slab_err(s, page, "Invalid object pointer 0x%p", object);
-		goto fail;
+		goto out;
 	}
 
 	if (on_freelist(s, page, object)) {
 		object_err(s, page, object, "Object already free");
-		goto fail;
+		goto out;
 	}
 
 	if (!check_object(s, page, object, SLUB_RED_ACTIVE))
@@ -1111,7 +1112,7 @@ next_object:
 		} else
 			object_err(s, page, object,
 					"page slab pointer corrupt.");
-		goto fail;
+		goto out;
 	}
 
 	if (s->flags & SLAB_STORE_USER)
@@ -1125,6 +1126,8 @@ next_object:
 		object = get_freepointer(s, object);
 		goto next_object;
 	}
+	ret = 1;
+
 out:
 	if (cnt != bulk_cnt)
 		slab_err(s, page, "Bulk freelist count(%d) invalid(%d)\n",
@@ -1132,13 +1135,9 @@ out:
 
 	slab_unlock(page);
 	spin_unlock_irqrestore(&n->list_lock, flags);
-	return 1;
-
-fail:
-	slab_unlock(page);
-	spin_unlock_irqrestore(&n->list_lock, flags);
-	slab_fix(s, "Object at 0x%p not freed", object);
-	return 0;
+	if (!ret)
+		slab_fix(s, "Object at 0x%p not freed", object);
+	return ret;
 }
 
 static int __init setup_slub_debug(char *str)
-- 
2.5.0

  parent reply	other threads:[~2016-02-15 18:44 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-02-15 18:44 [kernel-hardening] [PATCHv2 0/4] Improve performance for SLAB_POISON Laura Abbott
2016-02-15 18:44 ` [kernel-hardening] [PATCHv2 1/4] slub: Drop lock at the end of free_debug_processing Laura Abbott
2016-02-16 16:28   ` [kernel-hardening] " Christoph Lameter
2016-02-24 14:22   ` Paolo Bonzini
2016-02-24 18:09     ` Laura Abbott
2016-02-15 18:44 ` Laura Abbott [this message]
2016-02-16 16:30   ` [kernel-hardening] Re: [PATCHv2 2/4] slub: Fix/clean free_debug_processing return paths Christoph Lameter
2016-02-15 18:44 ` [kernel-hardening] [PATCHv2 3/4] slub: Convert SLAB_DEBUG_FREE to SLAB_CONSISTENCY_CHECKS Laura Abbott
2016-02-16 16:32   ` [kernel-hardening] " Christoph Lameter
2016-02-15 18:44 ` [kernel-hardening] [PATCHv2 4/4] slub: Relax CMPXCHG consistency restrictions Laura Abbott
2016-02-16 16:33   ` [kernel-hardening] " Christoph Lameter
2016-02-18  8:39 ` [kernel-hardening] Re: [PATCHv2 0/4] Improve performance for SLAB_POISON Joonsoo Kim

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1455561864-4217-3-git-send-email-labbott@fedoraproject.org \
    --to=labbott@fedoraproject.org \
    --cc=akpm@linux-foundation.org \
    --cc=cl@linux.com \
    --cc=js1304@gmail.com \
    --cc=keescook@chromium.org \
    --cc=kernel-hardening@lists.openwall.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=penberg@kernel.org \
    --cc=rientjes@google.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).