| Message ID | 1354812468-15709-2-git-send-email-johannes@sipsolutions.net (mailing list archive) |
|---|---|
| State | Not Applicable, archived |
| Headers | show |
On Thu, Dec 6, 2012 at 8:47 AM, Johannes Berg <johannes@sipsolutions.net> wrote: > From: Johannes Berg <johannes.berg@intel.com> > > When intersecting rules, we count first to know how many > rules need to be allocated, and then do the intersection > into the allocated array. However, the code doing this > writes past the end of the array because it attempts to > do all intersections. Make it stop when the right number > of rules has been reached. > > Cc: stable@vger.kernel.org > Signed-off-by: Johannes Berg <johannes.berg@intel.com> Acked-by: Luis R. Rodriguez <mcgrof@do-not-panic.com> A comment below though. > --- > net/wireless/reg.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/net/wireless/reg.c b/net/wireless/reg.c > index b6c7ea6..4197359 100644 > --- a/net/wireless/reg.c > +++ b/net/wireless/reg.c > @@ -648,9 +648,9 @@ static struct ieee80211_regdomain *regdom_intersect( > if (!rd) > return NULL; > > - for (x = 0; x < rd1->n_reg_rules; x++) { > + for (x = 0; x < rd1->n_reg_rules && rule_idx < num_rules; x++) { > rule1 = &rd1->reg_rules[x]; > - for (y = 0; y < rd2->n_reg_rules; y++) { > + for (y = 0; y < rd2->n_reg_rules && rule_idx < num_rules; y++) { > rule2 = &rd2->reg_rules[y]; > /* Does rule_idx ever become > num_rules though? The check that builds num_rules are the same as we traverse and increment rule_idx. Luis -- To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, 2012-12-06 at 15:43 -0800, Luis R. Rodriguez wrote: > > diff --git a/net/wireless/reg.c b/net/wireless/reg.c > > index b6c7ea6..4197359 100644 > > --- a/net/wireless/reg.c > > +++ b/net/wireless/reg.c > > @@ -648,9 +648,9 @@ static struct ieee80211_regdomain *regdom_intersect( > > if (!rd) > > return NULL; > > > > - for (x = 0; x < rd1->n_reg_rules; x++) { > > + for (x = 0; x < rd1->n_reg_rules && rule_idx < num_rules; x++) { > > rule1 = &rd1->reg_rules[x]; > > - for (y = 0; y < rd2->n_reg_rules; y++) { > > + for (y = 0; y < rd2->n_reg_rules && rule_idx < num_rules; y++) { > > rule2 = &rd2->reg_rules[y]; > > /* > > Does rule_idx ever become > num_rules though? The check that builds > num_rules are the same as we traverse and increment rule_idx. It doesn't become great, but it becomes equal. Say you have the following rules: rd1: 1000-2000, 3000-4000 rd2: 1000-1500, 5000-6000 The result will be 1000-1500, so 1 rule. But while iterating, that's the very first thing, so rule_idx becomes 1 after the first iteration of the inner/outer loops, and then without the fix we still check 1000-2000 vs. 5000-6000, 3000-4000 vs. 1000-1500 and finally 3000-4000 vs. 5000-6000 and rule_idx is 1 all the time while checking that so we write past the array ... This makes it stop when it knows it has found the right number of rules. johannes -- To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, 2012-12-06 at 17:47 +0100, Johannes Berg wrote: > From: Johannes Berg <johannes.berg@intel.com> > > When intersecting rules, we count first to know how many > rules need to be allocated, and then do the intersection > into the allocated array. However, the code doing this > writes past the end of the array because it attempts to > do all intersections. Make it stop when the right number > of rules has been reached. > > Cc: stable@vger.kernel.org > Signed-off-by: Johannes Berg <johannes.berg@intel.com> FWIW, since we currently allocate enough memory here to actually write past the end of the intended array, I've decided to remove the stable tag. It doesn't really fix anything -- with the next patch it fixes the allocation to not be too large, but that doesn't really need to go to stable. johannes -- To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, Dec 10, 2012 at 1:55 PM, Johannes Berg <johannes@sipsolutions.net> wrote: > On Thu, 2012-12-06 at 17:47 +0100, Johannes Berg wrote: >> From: Johannes Berg <johannes.berg@intel.com> >> >> When intersecting rules, we count first to know how many >> rules need to be allocated, and then do the intersection >> into the allocated array. However, the code doing this >> writes past the end of the array because it attempts to >> do all intersections. Make it stop when the right number >> of rules has been reached. >> >> Cc: stable@vger.kernel.org >> Signed-off-by: Johannes Berg <johannes.berg@intel.com> > > FWIW, since we currently allocate enough memory here to actually write > past the end of the intended array, I've decided to remove the stable > tag. It doesn't really fix anything -- with the next patch it fixes the > allocation to not be too large, but that doesn't really need to go to > stable. That was likely the issue I ran into that caused things to burp without the +1 ;) Luis -- To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/net/wireless/reg.c b/net/wireless/reg.c index b6c7ea6..4197359 100644 --- a/net/wireless/reg.c +++ b/net/wireless/reg.c @@ -648,9 +648,9 @@ static struct ieee80211_regdomain *regdom_intersect( if (!rd) return NULL; - for (x = 0; x < rd1->n_reg_rules; x++) { + for (x = 0; x < rd1->n_reg_rules && rule_idx < num_rules; x++) { rule1 = &rd1->reg_rules[x]; - for (y = 0; y < rd2->n_reg_rules; y++) { + for (y = 0; y < rd2->n_reg_rules && rule_idx < num_rules; y++) { rule2 = &rd2->reg_rules[y]; /* * This time around instead of using the stack lets