* [PATCH] jump_label: check entries limit in __jump_label_update @ 2011-05-04 15:30 Jiri Olsa 2011-05-09 19:32 ` Jason Baron 0 siblings, 1 reply; 6+ messages in thread From: Jiri Olsa @ 2011-05-04 15:30 UTC (permalink / raw) To: jbaron, rostedt, mingo; +Cc: linux-kernel, Jiri Olsa 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@redhat.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; } } @@ -241,11 +246,13 @@ static int jump_label_add_module(struct module *mod) jlm->mod = mod; jlm->entries = iter; + jlm->entries_stop = iter_stop; jlm->next = key->next; 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 +378,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 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] jump_label: check entries limit in __jump_label_update 2011-05-04 15:30 [PATCH] jump_label: check entries limit in __jump_label_update Jiri Olsa @ 2011-05-09 19:32 ` Jason Baron 2011-05-10 10:43 ` [PATCHv2] " Jiri Olsa 0 siblings, 1 reply; 6+ messages in thread From: Jason Baron @ 2011-05-09 19:32 UTC (permalink / raw) To: Jiri Olsa; +Cc: rostedt, mingo, linux-kernel 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@redhat.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? Otherwise, I agree this is a nice fix to have. Thanks, -Jason > } > } > @@ -241,11 +246,13 @@ static int jump_label_add_module(struct module *mod) > > jlm->mod = mod; > jlm->entries = iter; > + jlm->entries_stop = iter_stop; > jlm->next = key->next; > 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 +378,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 > ^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCHv2] jump_label: check entries limit in __jump_label_update 2011-05-09 19:32 ` Jason Baron @ 2011-05-10 10:43 ` Jiri Olsa 2011-05-10 14:30 ` Jason Baron 2011-05-27 12:46 ` [tip:perf/urgent] jump_label: Check " tip-bot for Jiri Olsa 0 siblings, 2 replies; 6+ messages in thread From: Jiri Olsa @ 2011-05-10 10:43 UTC (permalink / raw) To: Jason Baron; +Cc: rostedt, mingo, linux-kernel 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@redhat.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@redhat.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 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCHv2] jump_label: check entries limit in __jump_label_update 2011-05-10 10:43 ` [PATCHv2] " Jiri Olsa @ 2011-05-10 14:30 ` Jason Baron 2011-05-23 16:46 ` Jiri Olsa 2011-05-27 12:46 ` [tip:perf/urgent] jump_label: Check " tip-bot for Jiri Olsa 1 sibling, 1 reply; 6+ messages in thread From: Jason Baron @ 2011-05-10 14:30 UTC (permalink / raw) To: Jiri Olsa; +Cc: rostedt, mingo, linux-kernel On Tue, May 10, 2011 at 12:43:46PM +0200, Jiri Olsa wrote: > 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@redhat.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@redhat.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 > Looks good. Acked-by: Jason Baron <jbaron@redhat.com> Thanks, -Jason ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCHv2] jump_label: check entries limit in __jump_label_update 2011-05-10 14:30 ` Jason Baron @ 2011-05-23 16:46 ` Jiri Olsa 0 siblings, 0 replies; 6+ messages in thread From: Jiri Olsa @ 2011-05-23 16:46 UTC (permalink / raw) To: rostedt; +Cc: Jason Baron, mingo, linux-kernel hi, any feedback? thanks, jirka On Tue, May 10, 2011 at 10:30:45AM -0400, Jason Baron wrote: > On Tue, May 10, 2011 at 12:43:46PM +0200, Jiri Olsa wrote: > > 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@redhat.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@redhat.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 > > > > Looks good. > > Acked-by: Jason Baron <jbaron@redhat.com> > > Thanks, > > -Jason ^ permalink raw reply [flat|nested] 6+ messages in thread
* [tip:perf/urgent] jump_label: Check entries limit in __jump_label_update 2011-05-10 10:43 ` [PATCHv2] " Jiri Olsa 2011-05-10 14:30 ` Jason Baron @ 2011-05-27 12:46 ` tip-bot for Jiri Olsa 1 sibling, 0 replies; 6+ messages in thread From: tip-bot for Jiri Olsa @ 2011-05-27 12:46 UTC (permalink / raw) To: linux-tip-commits; +Cc: linux-kernel, hpa, mingo, rostedt, jbaron, tglx, jolsa Commit-ID: 7cbc5b8d4a775a43875a09e29c49a2a8195b5b2d Gitweb: http://git.kernel.org/tip/7cbc5b8d4a775a43875a09e29c49a2a8195b5b2d Author: Jiri Olsa <jolsa@redhat.com> AuthorDate: Tue, 10 May 2011 12:43:46 +0200 Committer: Steven Rostedt <rostedt@goodmis.org> CommitDate: Wed, 25 May 2011 19:56:36 -0400 jump_label: Check entries limit in __jump_label_update 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@redhat.com> Acked-by: Jason Baron <jbaron@redhat.com> Link: http://lkml.kernel.org/r/20110510104346.GC1899@jolsa.brq.redhat.com Signed-off-by: Steven Rostedt <rostedt@goodmis.org> --- 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); ^ permalink raw reply related [flat|nested] 6+ messages in thread
end of thread, other threads:[~2011-05-27 12:46 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2011-05-04 15:30 [PATCH] jump_label: check entries limit in __jump_label_update Jiri Olsa 2011-05-09 19:32 ` Jason Baron 2011-05-10 10:43 ` [PATCHv2] " Jiri Olsa 2011-05-10 14:30 ` Jason Baron 2011-05-23 16:46 ` Jiri Olsa 2011-05-27 12:46 ` [tip:perf/urgent] jump_label: Check " tip-bot for Jiri Olsa
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).