[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH XENSTORE v1 06/10] xenstored: handle port reads correctly

  • To: Norbert Manthey <nmanthey@xxxxxxxxx>, <xen-devel@xxxxxxxxxxxxxxxxxxxx>
  • From: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
  • Date: Fri, 26 Feb 2021 15:36:57 +0000
  • Arc-authentication-results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=citrix.com; dmarc=pass action=none header.from=citrix.com; dkim=pass header.d=citrix.com; arc=none
  • Arc-message-signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=07E0LxdgpUQqTVTGSukUDX52sFSOCZ0Rir1X2rkAvE0=; b=AvqkfzEqS4mIvQHCb0sQfiWK8ZvZwOY6C5baTFlCLPJQhdUbL/ZrdCuGulecc4sPTR2GYQY8lN3au511vFl5Izik8EsxJ+ypZZpjxAXkijj3Ud702sqR9eWlp6g4S9YZhO3Dvp3WAgKgZg9Vn74gqTSF9laeb27PwVBQwoTVzv3MAiI9yRuKkAqSTeliCQi1B0dYktCE8NYZP79EAWi+kYHxlJ2alRBkKxuOaGOON1AWw05oSSFvuKe9XCWgXZLocjhAO+SDflW3SwZVq2Yyw+imoYTW1RhiWgSP77Xm5WF6wDQtp7QE+F4sQU74NIYgksGqLOROchQ21NY349I82A==
  • Arc-seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=esYQY1feebpvIO2kIyg6b2SR3Wv3LYdwUJXhi6uVZs6LbJOSZIl9L2CQ820HGO4r7YEgtn9g7w5tXU0aYSnQC5WaS7f9xHP2BtQ8+FQE9/rOf778W+b8eV4r2InLWLNs8acvvYO56Oahs436gZpxAxkBzwVZTVQLNs79O+yAqBBMeHZB1hOoADevF8yMtqGlDEawb73a6E20ktXGdkgwJVhtekQ3h6+ak7jkPtumzP4Y+PpM3EsIgRgl19mtS8rA1JJ8AdXIypnL1Xa4oH7m1xALIHzKyhU+IqR9bJ/HMr2GJwC6GbdCoZh1Vl9H1ljCRVcVZD7PuEs+HjpkNAt/IA==
  • Authentication-results: esa2.hc3370-68.iphmx.com; dkim=pass (signature verified) header.i=@citrix.onmicrosoft.com
  • Cc: Ian Jackson <iwj@xxxxxxxxxxxxxx>, Juergen Gross <jgross@xxxxxxxx>, Wei Liu <wl@xxxxxxx>, Julien Grall <jgrall@xxxxxxxxxxxx>, Michael Kurth <mku@xxxxxxxxx>
  • Delivery-date: Fri, 26 Feb 2021 15:37:10 +0000
  • Ironport-sdr: stlaOehzP/NLdjGu6TWfEXkzrdYuz47NRhjCtgnHn7+7P5qWyAk2hdbY8DZz8Qc3wU4ZVkfvhC ToJeuM318K1Co/zLPPDBLQzXqHeeUSQzsac9HVNIVr7RtFXtoXg7SK8oh/WpQB67ajqA49rnbL JYVFCkANGV4P+glvRkZZCYGE/AGi+LxYiJdgX7J1li/m29nDikEASaMMjsMQYKqbQ6Lx97MKCi p2kD8TzJKxsFR0PVNrhee3t6DPrixsbljgSWWpFmqCLXk2KyEJJTOm5mX6a9Yetp4szca+ug3P diI=
  • List-id: Xen developer discussion <xen-devel.lists.xenproject.org>

On 26/02/2021 14:41, Norbert Manthey wrote:
> The read value could be larger than a signed 32bit integer. As -1 is
> used as error value, we should not rely on using the full 32 bits.
> Hence, when reading the port number, we should make sure we only return
> valid values.
> This change sanity checks the input.
> The issue is that the value for the port is
>  1. transmitted as a string, with a fixed amount of digits.
>  2. Next, this string is parsed by a function that can deal with strings
>     representing 64bit integers
>  3. A 64bit integer is returned, and will be truncated to it's lower
>     32bits, resulting in a wrong port number (in case the sender of the
>     string decides to craft a suitable 64bit value).
> The value is typically provided by the kernel, which has this value hard
> coded in the proper range. As we use the function strtoul, non-digit
> character are considered as end of the input, and hence do not require
> checking. Therefore, this change only covers the corner case to make
> sure we stay in the 32 bit range.
> This bug was discovered and resolved using Coverity Static Analysis
> Security Testing (SAST) by Synopsys, Inc.
> Signed-off-by: Norbert Manthey <nmanthey@xxxxxxxxx>
> Reviewed-by: Thomas Friebel <friebelt@xxxxxxxxx>
> Reviewed-by: Julien Grall <jgrall@xxxxxxxxxxxx>

Port numbers are currently limited at 2^17, with easy extension to 2^29
(iirc), but the entire event channel infrastructure would have to
undergo another redesign to get beyond that.

I think we can reasonably make an ABI statement saying that a port
number will never exceed 2^31.  This is already pseudo-encoded in the
evtchn_port_or_error_t mouthful.




Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.