linux-kernel - [PATCHv2] jump_label: check entries limit in __jump_label

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20110510104346.GC1899@jolsa.brq.redhat.com>
Date:	Tue, 10 May 2011 12:43:46 +0200
From:	Jiri Olsa <jolsa@...hat.com>
To:	Jason Baron <jbaron@...hat.com>
Cc:	rostedt@...dmis.org, mingo@...e.hu, linux-kernel@...r.kernel.org
Subject: [PATCHv2] jump_label: check entries limit in __jump_label_update

On Mon, May 09, 2011 at 03:32:48PM -0400, Jason Baron wrote:
> On Wed, May 04, 2011 at 05:30:23PM +0200, Jiri Olsa wrote:
> > When iterating the jump_label entries array (core or modules),
> > the __jump_label_update function peeks over the last entry.
> > 
> > The reason is that the end of the for loop depends on the key
> > value of the processed entry. Thus when going through the
> > last array entry, we will touch the memory behind the array
> > limit.
> > 
> > This bug probably will never be triggered, since most likely the
> > memory behind the jump_label entries will be accesable and the
> > entry->key will be different than the expected value.
> > 
> > 
> > Signed-off-by: Jiri Olsa <jolsa@...hat.com>
> > ---
> >  kernel/jump_label.c |   17 ++++++++++++-----
> >  1 files changed, 12 insertions(+), 5 deletions(-)
> > 
> > diff --git a/kernel/jump_label.c b/kernel/jump_label.c
> > index 74d1c09..b2ee97a 100644
> > --- a/kernel/jump_label.c
> > +++ b/kernel/jump_label.c
> > @@ -105,9 +105,12 @@ static int __jump_label_text_reserved(struct jump_entry *iter_start,
> >  }
> >  
> >  static void __jump_label_update(struct jump_label_key *key,
> > -		struct jump_entry *entry, int enable)
> > +				struct jump_entry *entry,
> > +				struct jump_entry *stop, int enable)
> >  {
> > -	for (; entry->key == (jump_label_t)(unsigned long)key; entry++) {
> > +	for (; (entry < stop) &&
> > +	      (entry->key == (jump_label_t)(unsigned long)key);
> > +	      entry++) {
> >  		/*
> >  		 * entry->code set to 0 invalidates module init text sections
> >  		 * kernel_text_address() verifies we are not in core kernel
> > @@ -158,6 +161,7 @@ early_initcall(jump_label_init);
> >  struct jump_label_mod {
> >  	struct jump_label_mod *next;
> >  	struct jump_entry *entries;
> > +	struct jump_entry *entries_stop;
> >  	struct module *mod;
> >  };
> >  
> > @@ -181,7 +185,8 @@ static void __jump_label_mod_update(struct jump_label_key *key, int enable)
> >  	struct jump_label_mod *mod = key->next;
> >  
> >  	while (mod) {
> > -		__jump_label_update(key, mod->entries, enable);
> > +		__jump_label_update(key, mod->entries, mod->entries_stop,
> > +				    enable);
> >  		mod = mod->next;
> 
> hmmm. Instead of adding a new field to the 'struct jump_label_mod' data
> structure (and thus increasing its footprint), why not use:
> mod->jump_entries +  mod->num_jump_entries here?

yep, overlooked the struct module pointer inside jump_label_mod
attaching new patch

thanks,
jirka
 
---
When iterating the jump_label entries array (core or modules),
the __jump_label_update function peeks over the last entry.

The reason is that the end of the for loop depends on the key
value of the processed entry. Thus when going through the
last array entry, we will touch the memory behind the array
limit.

This bug probably will never be triggered, since most likely the
memory behind the jump_label entries will be accesable and the
entry->key will be different than the expected value.


Signed-off-by: Jiri Olsa <jolsa@...hat.com>
---
 kernel/jump_label.c |   18 +++++++++++++-----
 1 files changed, 13 insertions(+), 5 deletions(-)

diff --git a/kernel/jump_label.c b/kernel/jump_label.c
index 74d1c09..fa27e75 100644
--- a/kernel/jump_label.c
+++ b/kernel/jump_label.c
@@ -105,9 +105,12 @@ static int __jump_label_text_reserved(struct jump_entry *iter_start,
 }
 
 static void __jump_label_update(struct jump_label_key *key,
-		struct jump_entry *entry, int enable)
+				struct jump_entry *entry,
+				struct jump_entry *stop, int enable)
 {
-	for (; entry->key == (jump_label_t)(unsigned long)key; entry++) {
+	for (; (entry < stop) &&
+	      (entry->key == (jump_label_t)(unsigned long)key);
+	      entry++) {
 		/*
 		 * entry->code set to 0 invalidates module init text sections
 		 * kernel_text_address() verifies we are not in core kernel
@@ -181,7 +184,11 @@ static void __jump_label_mod_update(struct jump_label_key *key, int enable)
 	struct jump_label_mod *mod = key->next;
 
 	while (mod) {
-		__jump_label_update(key, mod->entries, enable);
+		struct module *m = mod->mod;
+
+		__jump_label_update(key, mod->entries,
+				    m->jump_entries + m->num_jump_entries,
+				    enable);
 		mod = mod->next;
 	}
 }
@@ -245,7 +252,8 @@ static int jump_label_add_module(struct module *mod)
 		key->next = jlm;
 
 		if (jump_label_enabled(key))
-			__jump_label_update(key, iter, JUMP_LABEL_ENABLE);
+			__jump_label_update(key, iter, iter_stop,
+					    JUMP_LABEL_ENABLE);
 	}
 
 	return 0;
@@ -371,7 +379,7 @@ static void jump_label_update(struct jump_label_key *key, int enable)
 
 	/* if there are no users, entry can be NULL */
 	if (entry)
-		__jump_label_update(key, entry, enable);
+		__jump_label_update(key, entry, __stop___jump_table, enable);
 
 #ifdef CONFIG_MODULES
 	__jump_label_mod_update(key, enable);
-- 
1.7.1

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/