Received-SPF: pass (google.com: domain of linux-wireless-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Date:   Wed, 29 Apr 2020 11:10:34 -0700
From:   Jakub Kicinski <kuba@kernel.org>
To:     Johannes Berg <johannes@sipsolutions.net>
Cc:     netdev@vger.kernel.org, Antonio Quartulli <ordex@autistici.org>,
        linux-wireless@vger.kernel.org,
        Johannes Berg <johannes.berg@intel.com>
Subject: Re: [PATCH 4/7] netlink: extend policy range validation
Message-ID: <20200429111034.71ab2443@kicinski-fedora-pc1c0hjn.dhcp.thefacebook.com>
In-Reply-To: <20200429154836.b86f45043a5e.I7b46d9c85e4d7a99c0b5e0c2f54bb89b5750e6dc@changeid>
References: <20200429134843.42224-1-johannes@sipsolutions.net>
        <20200429154836.b86f45043a5e.I7b46d9c85e4d7a99c0b5e0c2f54bb89b5750e6dc@changeid>
MIME-Version: 1.0
Content-Type: text/plain; charset=US-ASCII
Content-Transfer-Encoding: 7bit
Sender: linux-wireless-owner@vger.kernel.org
Precedence: bulk

On Wed, 29 Apr 2020 15:48:40 +0200 Johannes Berg wrote:
> diff --git a/lib/nlattr.c b/lib/nlattr.c
> index 7f7ebd89caa4..bb66d06cc6f9 100644
> --- a/lib/nlattr.c
> +++ b/lib/nlattr.c
> @@ -111,17 +111,33 @@ static int nla_validate_array(const struct nlattr *head, int len, int maxtype,
>  	return 0;
>  }
>  
> -static int nla_validate_int_range(const struct nla_policy *pt,
> -				  const struct nlattr *nla,
> -				  struct netlink_ext_ack *extack)
> +static int nla_validate_int_range_unsigned(const struct nla_policy *pt,
> +					   const struct nlattr *nla,
> +					   struct netlink_ext_ack *extack)
>  {
> -	bool validate_min, validate_max;
> -	s64 value;
> +	struct netlink_range_validation _range = {
> +		.min = 0,
> +		.max = U64_MAX,
> +	}, *range = &_range;
> +	u64 value;
>  
> -	validate_min = pt->validation_type == NLA_VALIDATE_RANGE ||
> -		       pt->validation_type == NLA_VALIDATE_MIN;
> -	validate_max = pt->validation_type == NLA_VALIDATE_RANGE ||
> -		       pt->validation_type == NLA_VALIDATE_MAX;
> +	WARN_ON_ONCE(pt->min < 0 || pt->max < 0);

I'm probably missing something, but in case of NLA_VALIDATE_RANGE_PTR
aren't min and max invalid (union has the range pointer set, so this
will read 2 bytes of the pointer).

> +	switch (pt->validation_type) {
> +	case NLA_VALIDATE_RANGE:
> +		range->min = pt->min;
> +		range->max = pt->max;
> +		break;
> +	case NLA_VALIDATE_RANGE_PTR:
> +		range = pt->range;
> +		break;
> +	case NLA_VALIDATE_MIN:
> +		range->min = pt->min;
> +		break;
> +	case NLA_VALIDATE_MAX:
> +		range->max = pt->max;
> +		break;
> +	}