| Message ID | CAH7ZN-ziJXJQqzw5jr-DXf3h8Q0iOg49Vx0AsGdsj9Oz=7h+ag@mail.gmail.com |
|---|---|
| State | Accepted |
| Headers | show |
Mailer is doing something strange, patch is attached. On Tue, Feb 10, 2015 at 10:32 AM, Dmitry Shmidt <dimitrysh@google.com> wrote: > Change-Id: Ib9309b2cd461ac681bbc6e2efcec18b597e34f26 > Signed-off-by: Dmitry Shmidt <dimitrysh@google.com> > --- > wpa_supplicant/scan.c | 13 +++++++++---- > 1 file changed, 9 insertions(+), 4 deletions(-) > > diff --git a/wpa_supplicant/scan.c b/wpa_supplicant/scan.c > index debceb9..7c3d4db 100644 > --- a/wpa_supplicant/scan.c > +++ b/wpa_supplicant/scan.c > @@ -1944,7 +1944,7 @@ void wpa_scan_free_params(struct > wpa_driver_scan_params *params) > > int wpas_start_pno(struct wpa_supplicant *wpa_s) > { > - int ret, interval; > + int ret, interval, prio; > size_t i, num_ssid, num_match_ssid; > struct wpa_ssid *ssid; > struct wpa_driver_scan_params params; > @@ -2009,9 +2009,11 @@ int wpas_start_pno(struct wpa_supplicant *wpa_s) > sizeof(struct wpa_driver_scan_filter)); > if (params.filter_ssids == NULL) > return -1; > + > i = 0; > - ssid = wpa_s->conf->ssid; > - while (ssid) { > + prio = 0; > + ssid = wpa_s->conf->pssid[prio]; > + while (ssid && (prio < wpa_s->conf->num_prio)) { > if (!wpas_network_disabled(wpa_s, ssid)) { > if (ssid->scan_ssid && params.num_ssids < num_ssid) { > params.ssids[params.num_ssids].ssid = > @@ -2028,7 +2030,10 @@ int wpas_start_pno(struct wpa_supplicant *wpa_s) > if (i == num_match_ssid) > break; > } > - ssid = ssid->next; > + if (ssid->pnext) > + ssid = ssid->pnext; > + else > + ssid = wpa_s->conf->pssid[++prio]; > } > > if (wpa_s->conf->filter_rssi) > -- > 2.2.0.rc0.207.ga3a616c
On Tue, Feb 10, 2015 at 10:32:23AM -0800, Dmitry Shmidt wrote: > diff --git a/wpa_supplicant/scan.c b/wpa_supplicant/scan.c > @@ -2009,9 +2009,11 @@ int wpas_start_pno(struct wpa_supplicant *wpa_s) > sizeof(struct wpa_driver_scan_filter)); > i = 0; > - ssid = wpa_s->conf->ssid; > - while (ssid) { > + prio = 0; > + ssid = wpa_s->conf->pssid[prio]; > + while (ssid && (prio < wpa_s->conf->num_prio)) { > @@ -2028,7 +2030,10 @@ int wpas_start_pno(struct wpa_supplicant *wpa_s) > - ssid = ssid->next; > + if (ssid->pnext) > + ssid = ssid->pnext; > + else > + ssid = wpa_s->conf->pssid[++prio]; It looks like other cases of iterating through ssid->pnext pointers are using a separate for loop to go through all the prio values (e.g., see wpa_supplicant_pick_new_network()). Those would allow the wpa_s->conf->pssid[] array to have a NULL pointer in it. This does not seem to happen currently, so this is not of that much concern on its own and the single loop here could be made to work. However, it looks like the final loop through this would read beyond the end of the pssid[] array. The while condition would stop that ssid pointer from being used, but if I understood the implementation here correctly, that pssid[++prio] would need to be protected with something like if (ssid->pnext) ssid = ssid->pnext; else if (prio + 1 == wpa_s->conf->num_prio) break; else ssid = wpa_s->conf->pssid[++prio]; to avoid potential issues (and analyzer warnings) on reading one pointer beyond the allocation of the pssid[] array.
On Thu, Feb 12, 2015 at 5:29 AM, Jouni Malinen <j@w1.fi> wrote: > On Tue, Feb 10, 2015 at 10:32:23AM -0800, Dmitry Shmidt wrote: >> diff --git a/wpa_supplicant/scan.c b/wpa_supplicant/scan.c >> @@ -2009,9 +2009,11 @@ int wpas_start_pno(struct wpa_supplicant *wpa_s) >> sizeof(struct wpa_driver_scan_filter)); >> i = 0; >> - ssid = wpa_s->conf->ssid; >> - while (ssid) { >> + prio = 0; >> + ssid = wpa_s->conf->pssid[prio]; >> + while (ssid && (prio < wpa_s->conf->num_prio)) { > >> @@ -2028,7 +2030,10 @@ int wpas_start_pno(struct wpa_supplicant *wpa_s) >> - ssid = ssid->next; >> + if (ssid->pnext) >> + ssid = ssid->pnext; >> + else >> + ssid = wpa_s->conf->pssid[++prio]; > > It looks like other cases of iterating through ssid->pnext pointers are > using a separate for loop to go through all the prio values (e.g., see > wpa_supplicant_pick_new_network()). Those would allow the > wpa_s->conf->pssid[] array to have a NULL pointer in it. This does not > seem to happen currently, so this is not of that much concern on its own > and the single loop here could be made to work. However, it looks like > the final loop through this would read beyond the end of the pssid[] > array. The while condition would stop that ssid pointer from being used, > but if I understood the implementation here correctly, that > pssid[++prio] would need to be protected with something like > > if (ssid->pnext) > ssid = ssid->pnext; > else if (prio + 1 == wpa_s->conf->num_prio) > break; > else > ssid = wpa_s->conf->pssid[++prio]; > > to avoid potential issues (and analyzer warnings) on reading one pointer > beyond the allocation of the pssid[] array. Thank you for catching this. New patch is attached. > > -- > Jouni Malinen PGP id EFC895FA > _______________________________________________ > HostAP mailing list > HostAP@lists.shmoo.com > http://lists.shmoo.com/mailman/listinfo/hostap
On Thu, Feb 12, 2015 at 10:27:33AM -0800, Dmitry Shmidt wrote:
> Thank you for catching this. New patch is attached.
Thanks, applied.
diff --git a/wpa_supplicant/scan.c b/wpa_supplicant/scan.c index debceb9..7c3d4db 100644 --- a/wpa_supplicant/scan.c +++ b/wpa_supplicant/scan.c @@ -1944,7 +1944,7 @@ void wpa_scan_free_params(struct wpa_driver_scan_params *params) int wpas_start_pno(struct wpa_supplicant *wpa_s) { - int ret, interval; + int ret, interval, prio; size_t i, num_ssid, num_match_ssid; struct wpa_ssid *ssid;
Change-Id: Ib9309b2cd461ac681bbc6e2efcec18b597e34f26 Signed-off-by: Dmitry Shmidt <dimitrysh@google.com> --- wpa_supplicant/scan.c | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) struct wpa_driver_scan_params params; @@ -2009,9 +2009,11 @@ int wpas_start_pno(struct wpa_supplicant *wpa_s) sizeof(struct wpa_driver_scan_filter)); if (params.filter_ssids == NULL) return -1; + i = 0; - ssid = wpa_s->conf->ssid; - while (ssid) { + prio = 0; + ssid = wpa_s->conf->pssid[prio]; + while (ssid && (prio < wpa_s->conf->num_prio)) { if (!wpas_network_disabled(wpa_s, ssid)) { if (ssid->scan_ssid && params.num_ssids < num_ssid) { params.ssids[params.num_ssids].ssid = @@ -2028,7 +2030,10 @@ int wpas_start_pno(struct wpa_supplicant *wpa_s) if (i == num_match_ssid) break; } - ssid = ssid->next; + if (ssid->pnext) + ssid = ssid->pnext; + else + ssid = wpa_s->conf->pssid[++prio]; } if (wpa_s->conf->filter_rssi) -- 2.2.0.rc0.207.ga3a616c