Re: [PATCH wpan-next 1/6] ieee802154: Add support for user scanning requests

[Jakub Kicinski <kuba@kernel.org>]

On Tue, 29 Nov 2022 17:00:41 +0100 Miquel Raynal wrote:
> +static int nl802154_trigger_scan(struct sk_buff *skb, struct genl_info *info)
> +{
> +	struct cfg802154_registered_device *rdev = info->user_ptr[0];
> +	struct net_device *dev = info->user_ptr[1];
> +	struct wpan_dev *wpan_dev = dev->ieee802154_ptr;
> +	struct wpan_phy *wpan_phy = &rdev->wpan_phy;
> +	struct cfg802154_scan_request *request;
> +	u8 type;
> +	int err;
> +
> +	/* Monitors are not allowed to perform scans */
> +	if (wpan_dev->iftype == NL802154_IFTYPE_MONITOR)

extack ?

> +		return -EPERM;
> +
> +	request = kzalloc(sizeof(*request), GFP_KERNEL);
> +	if (!request)
> +		return -ENOMEM;
> +
> +	request->wpan_dev = wpan_dev;
> +	request->wpan_phy = wpan_phy;
> +
> +	type = nla_get_u8(info->attrs[NL802154_ATTR_SCAN_TYPE]);

what checks info->attrs[NL802154_ATTR_SCAN_TYPE] is not NULL?

> +	switch (type) {
> +	case NL802154_SCAN_PASSIVE:
> +		request->type = type;
> +		break;
> +	default:
> +		pr_err("Unsupported scan type: %d\n", type);
> +		err = -EINVAL;

extack (printfs are now supported)

> +		goto free_request;
> +	}
> +
> +	if (info->attrs[NL802154_ATTR_PAGE]) {
> +		request->page = nla_get_u8(info->attrs[NL802154_ATTR_PAGE]);
> +		if (request->page > IEEE802154_MAX_PAGE) {

bound check should be part of the policy NLA_POLICY_MAX()

> +			pr_err("Invalid page %d > %d\n",
> +			       request->page, IEEE802154_MAX_PAGE);
> +			err = -EINVAL;

extack

> +			goto free_request;
> +		}
> +	} else {
> +		/* Use current page by default */
> +		request->page = wpan_phy->current_page;
> +	}
> +
> +	if (info->attrs[NL802154_ATTR_SCAN_CHANNELS]) {
> +		request->channels = nla_get_u32(info->attrs[NL802154_ATTR_SCAN_CHANNELS]);
> +		if (request->channels >= BIT(IEEE802154_MAX_CHANNEL + 1)) {

policy as well

> +			pr_err("Invalid channels bitfield %x ≥ %lx\n",
> +			       request->channels,
> +			       BIT(IEEE802154_MAX_CHANNEL + 1));
> +			err = -EINVAL;
> +			goto free_request;
> +		}
> +	} else {
> +		/* Scan all supported channels by default */
> +		request->channels = wpan_phy->supported.channels[request->page];
> +	}
> +
> +	if (info->attrs[NL802154_ATTR_SCAN_PREAMBLE_CODES] ||
> +	    info->attrs[NL802154_ATTR_SCAN_MEAN_PRF]) {
> +		pr_err("Preamble codes and mean PRF not supported yet\n");

NLA_REJECT also in policy

> +		err = -EINVAL;
> +		goto free_request;
> +	}
> +
> +	if (info->attrs[NL802154_ATTR_SCAN_DURATION]) {
> +		request->duration = nla_get_u8(info->attrs[NL802154_ATTR_SCAN_DURATION]);
> +		if (request->duration > IEEE802154_MAX_SCAN_DURATION) {
> +			pr_err("Duration is out of range\n");
> +			err = -EINVAL;
> +			goto free_request;
> +		}
> +	} else {
> +		/* Use maximum duration order by default */
> +		request->duration = IEEE802154_MAX_SCAN_DURATION;
> +	}
> +
> +	if (wpan_dev->netdev)
> +		dev_hold(wpan_dev->netdev);

Can we put a tracker in the request and use netdev_hold() ?

> +
> +	err = rdev_trigger_scan(rdev, request);
> +	if (err) {
> +		pr_err("Failure starting scanning (%d)\n", err);
> +		goto free_device;
> +	}