| Message ID | 1345076116-5053-1-git-send-email-c_tpeder@qca.qualcomm.com (mailing list archive) |
|---|---|
| State | Not Applicable, archived |
| Headers | show |
On 08/16/2012 03:15 AM, Thomas Pedersen wrote: > Don't accept WoW patterns longer than supported by firmware. > > Reported-by: Haijun Jin <nhjin@qca.qualcomm.com> > Signed-off-by: Thomas Pedersen <c_tpeder@qca.qualcomm.com> Thanks, applied. Kalle -- 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 Wed, 2012-08-15 at 17:15 -0700, Thomas Pedersen wrote: > Don't accept WoW patterns longer than supported by firmware. > > Reported-by: Haijun Jin <nhjin@qca.qualcomm.com> > Signed-off-by: Thomas Pedersen <c_tpeder@qca.qualcomm.com> > --- > drivers/net/wireless/ath/ath6kl/cfg80211.c | 3 +++ > 1 files changed, 3 insertions(+), 0 deletions(-) > > diff --git a/drivers/net/wireless/ath/ath6kl/cfg80211.c b/drivers/net/wireless/ath/ath6kl/cfg80211.c > index bd003fe..ffa18f3 100644 > --- a/drivers/net/wireless/ath/ath6kl/cfg80211.c > +++ b/drivers/net/wireless/ath/ath6kl/cfg80211.c > @@ -1876,6 +1876,9 @@ static int ath6kl_wow_usr(struct ath6kl *ar, struct ath6kl_vif *vif, > /* Configure the patterns that we received from the user. */ > for (i = 0; i < wow->n_patterns; i++) { > > + if (wow->patterns[i].pattern_len > WOW_MASK_SIZE) > + return -EINVAL; > + No objection, but doesn't nl80211 already validate that (assuming you give the right pattern_max_len, of course)? 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 08/20/2012 10:13 AM, Johannes Berg wrote: > On Wed, 2012-08-15 at 17:15 -0700, Thomas Pedersen wrote: >> Don't accept WoW patterns longer than supported by firmware. >> >> Reported-by: Haijun Jin <nhjin@qca.qualcomm.com> >> Signed-off-by: Thomas Pedersen <c_tpeder@qca.qualcomm.com> >> --- >> drivers/net/wireless/ath/ath6kl/cfg80211.c | 3 +++ >> 1 files changed, 3 insertions(+), 0 deletions(-) >> >> diff --git a/drivers/net/wireless/ath/ath6kl/cfg80211.c b/drivers/net/wireless/ath/ath6kl/cfg80211.c >> index bd003fe..ffa18f3 100644 >> --- a/drivers/net/wireless/ath/ath6kl/cfg80211.c >> +++ b/drivers/net/wireless/ath/ath6kl/cfg80211.c >> @@ -1876,6 +1876,9 @@ static int ath6kl_wow_usr(struct ath6kl *ar, struct ath6kl_vif *vif, >> /* Configure the patterns that we received from the user. */ >> for (i = 0; i < wow->n_patterns; i++) { >> >> + if (wow->patterns[i].pattern_len > WOW_MASK_SIZE) >> + return -EINVAL; >> + > > No objection, but doesn't nl80211 already validate that (assuming you > give the right pattern_max_len, of course)? And ath6kl even uses different define pattern_max_len: wiphy->wowlan.pattern_max_len = WOW_PATTERN_SIZE; But the value is still same: #define WOW_PATTERN_SIZE 64 #define WOW_MASK_SIZE 64 Thomas, can you please check this? Do we really need two different defines? And which one is the correct one here? I'll keep the patch applied but I'm happy to take followup patches to clarify this part. Kalle -- 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, Aug 20, 2012 at 10:29:19AM +0300, Kalle Valo wrote: > On 08/20/2012 10:13 AM, Johannes Berg wrote: > > On Wed, 2012-08-15 at 17:15 -0700, Thomas Pedersen wrote: > >> Don't accept WoW patterns longer than supported by firmware. > >> > >> Reported-by: Haijun Jin <nhjin@qca.qualcomm.com> > >> Signed-off-by: Thomas Pedersen <c_tpeder@qca.qualcomm.com> > >> --- > >> drivers/net/wireless/ath/ath6kl/cfg80211.c | 3 +++ > >> 1 files changed, 3 insertions(+), 0 deletions(-) > >> > >> diff --git a/drivers/net/wireless/ath/ath6kl/cfg80211.c b/drivers/net/wireless/ath/ath6kl/cfg80211.c > >> index bd003fe..ffa18f3 100644 > >> --- a/drivers/net/wireless/ath/ath6kl/cfg80211.c > >> +++ b/drivers/net/wireless/ath/ath6kl/cfg80211.c > >> @@ -1876,6 +1876,9 @@ static int ath6kl_wow_usr(struct ath6kl *ar, struct ath6kl_vif *vif, > >> /* Configure the patterns that we received from the user. */ > >> for (i = 0; i < wow->n_patterns; i++) { > >> > >> + if (wow->patterns[i].pattern_len > WOW_MASK_SIZE) > >> + return -EINVAL; > >> + > > > > No objection, but doesn't nl80211 already validate that (assuming you > > give the right pattern_max_len, of course)? Thanks for pointing that out. That check would be completely redundant then. Kalle, Can you revert this patch? Otherwise the followup will just do the same. > And ath6kl even uses different define pattern_max_len: > > wiphy->wowlan.pattern_max_len = WOW_PATTERN_SIZE; > > But the value is still same: > > #define WOW_PATTERN_SIZE 64 > #define WOW_MASK_SIZE 64 > > Thomas, can you please check this? Do we really need two different > defines? And which one is the correct one here? No AFAICT there is no reason to have two different defines. I can submit a small patch consolidating these, but it would remove the above hunk anyway so I need to know whether you'll revert or not. Thanks, Thomas -- 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 08/20/2012 09:18 PM, Pedersen, Thomas wrote: > On Mon, Aug 20, 2012 at 10:29:19AM +0300, Kalle Valo wrote: >> On 08/20/2012 10:13 AM, Johannes Berg wrote: >> >>> No objection, but doesn't nl80211 already validate that (assuming you >>> give the right pattern_max_len, of course)? > > Thanks for pointing that out. That check would be completely redundant > then. > > Kalle, > > Can you revert this patch? Otherwise the followup will just do the same. I can revert the patch. But IMHO the check isn't that bad, and even cfg80211 can be buggy sometimes ;) >> And ath6kl even uses different define pattern_max_len: >> >> wiphy->wowlan.pattern_max_len = WOW_PATTERN_SIZE; >> >> But the value is still same: >> >> #define WOW_PATTERN_SIZE 64 >> #define WOW_MASK_SIZE 64 >> >> Thomas, can you please check this? Do we really need two different >> defines? And which one is the correct one here? > > No AFAICT there is no reason to have two different defines. I can submit > a small patch consolidating these, but it would remove the above hunk > anyway so I need to know whether you'll revert or not. Thanks. I'll revert the patch so please prepare your patch without the check. Kalle -- 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, Aug 20, 2012 at 10:08:47PM +0300, Kalle Valo wrote: > > Can you revert this patch? Otherwise the followup will just do the same. > > I can revert the patch. But IMHO the check isn't that bad, and even > cfg80211 can be buggy sometimes ;) > Well it's probably better not to cover any cfg80211 bugs up in the driver anyway. > >> And ath6kl even uses different define pattern_max_len: > >> > >> wiphy->wowlan.pattern_max_len = WOW_PATTERN_SIZE; > >> > >> But the value is still same: > >> > >> #define WOW_PATTERN_SIZE 64 > >> #define WOW_MASK_SIZE 64 > >> > >> Thomas, can you please check this? Do we really need two different > >> defines? And which one is the correct one here? > > > > No AFAICT there is no reason to have two different defines. I can submit > > a small patch consolidating these, but it would remove the above hunk > > anyway so I need to know whether you'll revert or not. > > Thanks. I'll revert the patch so please prepare your patch without the > check. OK. Thomas -- 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/drivers/net/wireless/ath/ath6kl/cfg80211.c b/drivers/net/wireless/ath/ath6kl/cfg80211.c index bd003fe..ffa18f3 100644 --- a/drivers/net/wireless/ath/ath6kl/cfg80211.c +++ b/drivers/net/wireless/ath/ath6kl/cfg80211.c @@ -1876,6 +1876,9 @@ static int ath6kl_wow_usr(struct ath6kl *ar, struct ath6kl_vif *vif, /* Configure the patterns that we received from the user. */ for (i = 0; i < wow->n_patterns; i++) { + if (wow->patterns[i].pattern_len > WOW_MASK_SIZE) + return -EINVAL; + /* * Convert given nl80211 specific mask value to equivalent * driver specific mask value and send it to the chip along
Don't accept WoW patterns longer than supported by firmware. Reported-by: Haijun Jin <nhjin@qca.qualcomm.com> Signed-off-by: Thomas Pedersen <c_tpeder@qca.qualcomm.com> --- drivers/net/wireless/ath/ath6kl/cfg80211.c | 3 +++ 1 files changed, 3 insertions(+), 0 deletions(-)