* Re: [PATCH] staging: rtl8188eu: Fix off-by-one in core/rtw_mlme_ext.c
[not found] <HK0PR01MB216364A68D785B7EBC8DD43EADD00@HK0PR01MB2163.apcprd01.prod.exchangelabs.com>
@ 2018-11-27 7:57 ` gregkh
2018-11-27 8:15 ` Dan Carpenter
1 sibling, 0 replies; 4+ messages in thread
From: gregkh @ 2018-11-27 7:57 UTC (permalink / raw)
To: Yang Xiao
Cc: r.weclawski, straube.linux, santhameena13, devel, jananis37,
linux-kernel
On Tue, Nov 27, 2018 at 07:29:07AM +0000, Yang Xiao wrote:
> From: Young_X <YangX92@hotmail.com>
Please use your "full" name like you did in your from: line of your
email
>
> The error at line 3267 was the result of an off-by-one error in
> a for loop in line 3253.
> If condition in line 3254 never satisfies, then the value of
> pstat->aid is NUM_STA+1. This will lead to out-of-bound access
> in line 3267.
Why is this indented? No need for that.
Can you fix that up and resend?
thanks,
greg k-h
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] staging: rtl8188eu: Fix off-by-one in core/rtw_mlme_ext.c
[not found] <HK0PR01MB216364A68D785B7EBC8DD43EADD00@HK0PR01MB2163.apcprd01.prod.exchangelabs.com>
2018-11-27 7:57 ` [PATCH] staging: rtl8188eu: Fix off-by-one in core/rtw_mlme_ext.c gregkh
@ 2018-11-27 8:15 ` Dan Carpenter
[not found] ` <HK0PR01MB2163A0287797ECA3CDA68AA2ADD00@HK0PR01MB2163.apcprd01.prod.exchangelabs.com>
1 sibling, 1 reply; 4+ messages in thread
From: Dan Carpenter @ 2018-11-27 8:15 UTC (permalink / raw)
To: Yang Xiao
Cc: gregkh, r.weclawski, straube.linux, santhameena13, devel,
jananis37, linux-kernel
The original code is OK.
On Tue, Nov 27, 2018 at 07:29:07AM +0000, Yang Xiao wrote:
> From: Young_X <YangX92@hotmail.com>
>
> The error at line 3267 was the result of an off-by-one error in
> a for loop in line 3253.
> If condition in line 3254 never satisfies, then the value of
> pstat->aid is NUM_STA+1. This will lead to out-of-bound access
> in line 3267.
It's best to avoid line numbers in the commit message unless you paste
the actual code, because sometimes people will be using different line
numbers from you.
> Signed-off-by: Young_X <YangX92@hotmail.com>
> ---
> drivers/staging/rtl8188eu/core/rtw_mlme_ext.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/staging/rtl8188eu/core/rtw_mlme_ext.c b/drivers/staging/rtl8188eu/core/rtw_mlme_ext.c
> index 6790b840..0854adc 100644
> --- a/drivers/staging/rtl8188eu/core/rtw_mlme_ext.c
> +++ b/drivers/staging/rtl8188eu/core/rtw_mlme_ext.c
> @@ -3250,7 +3250,7 @@ static unsigned int OnAssocReq(struct adapter *padapter,
> if (pstat->aid > 0) {
> DBG_88E(" old AID %d\n", pstat->aid);
> } else {
> - for (pstat->aid = 1; pstat->aid <= NUM_STA; pstat->aid++)
> + for (pstat->aid = 1; pstat->aid < NUM_STA; pstat->aid++)
drivers/staging/rtl8188eu/core/rtw_mlme_ext.c
3249 /* get a unique AID */
3250 if (pstat->aid > 0) {
3251 DBG_88E(" old AID %d\n", pstat->aid);
3252 } else {
3253 for (pstat->aid = 1; pstat->aid <= NUM_STA; pstat->aid++)
3254 if (pstapriv->sta_aid[pstat->aid - 1] == NULL)
3255 break;
3256
3257 /* if (pstat->aid > NUM_STA) { */
^^^^^^^^^^^^^^^^^^^^^^^^
This comment is key. pstapriv->max_num_sta is always less than or equal
to NUM_STA.
3258 if (pstat->aid > pstapriv->max_num_sta) {
3259 pstat->aid = 0;
3260
3261 DBG_88E(" no room for more AIDs\n");
3262
3263 status = WLAN_STATUS_AP_UNABLE_TO_HANDLE_NEW_STA;
3264
3265 goto OnAssocReqFail;
3266 } else {
3267 pstapriv->sta_aid[pstat->aid - 1] = pstat;
^^^^^^^^^^^^^^
So this is fine.
3268 DBG_88E("allocate new AID=(%d)\n", pstat->aid);
3269 }
3270 }
3271
regards,
dan carpenter
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] staging: rtl8188eu: Fix off-by-one in core/rtw_mlme_ext.c
[not found] ` <HK0PR01MB2163A0287797ECA3CDA68AA2ADD00@HK0PR01MB2163.apcprd01.prod.exchangelabs.com>
@ 2018-11-27 8:34 ` Dan Carpenter
[not found] ` <HK0PR01MB2163FB565C3ADD4BCE1FE7D2ADD00@HK0PR01MB2163.apcprd01.prod.exchangelabs.com>
0 siblings, 1 reply; 4+ messages in thread
From: Dan Carpenter @ 2018-11-27 8:34 UTC (permalink / raw)
To: Yang Xiao
Cc: gregkh, r.weclawski, straube.linux, santhameena13, devel,
jananis37, linux-kernel
On Tue, Nov 27, 2018 at 08:29:05AM +0000, Yang Xiao wrote:
> Hi,
>
> See commit ef9209b642f ("staging: rtl8723bs: Fix indenting errors and an
> off-by-one mistake in core/rtw_mlme_ext.c") for detail.
>
> I don't know how can you make sure that line 3254 can be true in the for
> loop. If the condition never satisfies, then there is an off-by-one
> access in line 3267.
>
> If you can prove it, then the patch is unnecessary.
>
Ugh... Patch ef9209b642f isn't right. :(
Do you want to send a patch to change that back to the way it was. If
not then I can do it.
regards,
dan carpenter
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] staging: rtl8188eu: Fix off-by-one in core/rtw_mlme_ext.c
[not found] ` <HK0PR01MB2163FB565C3ADD4BCE1FE7D2ADD00@HK0PR01MB2163.apcprd01.prod.exchangelabs.com>
@ 2018-11-27 8:49 ` Dan Carpenter
0 siblings, 0 replies; 4+ messages in thread
From: Dan Carpenter @ 2018-11-27 8:49 UTC (permalink / raw)
To: Yang Xiao
Cc: devel, gregkh, linux-kernel, jananis37, r.weclawski, santhameena13
On Tue, Nov 27, 2018 at 08:41:53AM +0000, Yang Xiao wrote:
> Okay. I can send a patch to revert ef9209b642f.
>
> But, can you make sure that the condition "(pstapriv->sta_aid[pstat->aid
> - 1] == NULL)" can satisfies in the for loop?
->max_num_sta is either set in _rtw_init_sta_priv() or rtw_set_beacon().
You can see that it's <= MAX_STA.
regards,
dan carpenter
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2018-11-27 8:50 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
[not found] <HK0PR01MB216364A68D785B7EBC8DD43EADD00@HK0PR01MB2163.apcprd01.prod.exchangelabs.com>
2018-11-27 7:57 ` [PATCH] staging: rtl8188eu: Fix off-by-one in core/rtw_mlme_ext.c gregkh
2018-11-27 8:15 ` Dan Carpenter
[not found] ` <HK0PR01MB2163A0287797ECA3CDA68AA2ADD00@HK0PR01MB2163.apcprd01.prod.exchangelabs.com>
2018-11-27 8:34 ` Dan Carpenter
[not found] ` <HK0PR01MB2163FB565C3ADD4BCE1FE7D2ADD00@HK0PR01MB2163.apcprd01.prod.exchangelabs.com>
2018-11-27 8:49 ` Dan Carpenter
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.