On Thu, 2009-03-19 at 13:39 +0200, Jouni Malinen wrote:
> + if (info->attrs[NL80211_ATTR_WIPHY_FREQ]) {
> + req.chan = ieee80211_get_channel(
> + wiphy,
> + nla_get_u32(info->attrs[NL80211_ATTR_WIPHY_FREQ]));
> + if (!req.chan) {
> + err = -EINVAL;
> + goto out;
> + }
> + }
Validate the channel is permitted?
> + if (info->attrs[NL80211_ATTR_AUTH_TYPE]) {
> + req.auth_type =
> + nla_get_u32(info->attrs[NL80211_ATTR_AUTH_TYPE]);
> + }
We should validate the auth type is one of the four valid ones, I think.
> + if (nla_len(info->attrs[NL80211_ATTR_SSID]) > IEEE80211_MAX_SSID_LEN) {
> + err = -EINVAL;
> + goto out;
> + }
That seems unnecessary since your netlink policy restricts it already.
> + if (info->attrs[NL80211_ATTR_REASON_CODE])
> + req.reason_code =
> + nla_get_u16(info->attrs[NL80211_ATTR_REASON_CODE]);
Should probably require non-zero? Same in disassoc, I guess.
Since disassoc and deauth are really almost the same, maybe use one
function and multiplex like nl80211_addset_beacon?
> +static int ieee80211_auth(struct wiphy *wiphy, struct net_device *dev,
> + struct cfg80211_auth_request *req)
> +{
> + struct ieee80211_sub_if_data *sdata;
> +
> + if (!netif_running(dev))
> + return -ENETDOWN;
Should probably be in cfg80211 directly, I think, to enforce this across
drivers.
> + sdata = IEEE80211_DEV_TO_SUB_IF(dev);
> +
> + if (sdata->vif.type != NL80211_IFTYPE_STATION)
> + return -EOPNOTSUPP;
That too.
> + switch (req->auth_type) {
> + case NL80211_AUTHTYPE_OPEN_SYSTEM:
> + sdata->u.mgd.auth_algs = IEEE80211_AUTH_ALG_OPEN;
> + break;
> + case NL80211_AUTHTYPE_SHARED_KEY:
> + sdata->u.mgd.auth_algs = IEEE80211_AUTH_ALG_SHARED_KEY;
> + break;
> + case NL80211_AUTHTYPE_FT:
> + sdata->u.mgd.auth_algs = IEEE80211_AUTH_ALG_FT;
> + break;
> + case NL80211_AUTHTYPE_NETWORK_EAP:
> + sdata->u.mgd.auth_algs = IEEE80211_AUTH_ALG_LEAP;
> + break;
> + default:
> + return -EOPNOTSUPP;
and I already said the "default" part here should be too (though it
should stick around here if we ever add new mechanisms)
Similarly in assoc, of course.
otherwise looks good.
johannes
Attachment:
signature.asc
Description: This is a digitally signed message part
