On 9 June 2014 18:27, Eliad Peller [off-list ref] wrote:

hi Michal,

On Thu, Jun 5, 2014 at 3:56 PM, Michal Kazior [off-list ref] wrote:

quoted

Multi-vif in-place reservations happen when
it is impossible to allocate more channel contexts
as indicated by interface combinations.

Such reservations are not finalized until all
assigned interfaces are ready.

This still doesn't handle all possible cases
(i.e. degradation of number of channels) properly.

Signed-off-by: Michal Kazior <redacted>
---

[...]

quoted

+static int
+ieee80211_vif_use_reserved_reassign(struct ieee80211_sub_if_data *sdata)
 {

[...]

quoted

+
+       vif_chsw[0].vif = &sdata->vif;
+       vif_chsw[0].old_ctx = &old_ctx->conf;
+       vif_chsw[0].new_ctx = &new_ctx->conf;
+
+       list_del(&sdata->reserved_chanctx_list);
+       sdata->reserved_chanctx = NULL;
+
+       err = drv_switch_vif_chanctx(local, vif_chsw, 1,
+                                    CHANCTX_SWMODE_REASSIGN_VIF);
+       if (err) {
+               if (ieee80211_chanctx_refcount(local, new_ctx) == 0)
+                       ieee80211_free_chanctx(local, new_ctx);
+
+               return err;
        }

-       old_ctx = container_of(conf, struct ieee80211_chanctx, conf);
+       list_move(&sdata->assigned_chanctx_list, &new_ctx->assigned_vifs);
+
+       if (sdata->vif.type == NL80211_IFTYPE_AP)
+               __ieee80211_vif_copy_chanctx_to_vlans(sdata, false);
+
+       if (ieee80211_chanctx_refcount(local, old_ctx) == 0)
+               ieee80211_free_chanctx(local, old_ctx);

i gave it a quick run, and it crashed quickly due to use-after-free.
adding:
rcu_assign_pointer(sdata->vif.chanctx_conf, &new_ctx->conf);

after the list_move (before freeing it...) seemed to solve it.
other than that, it seems to work well so far :)

Good catch! Thanks!


Michał