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 <nmant...@amazon.de> > Reviewed-by: Thomas Friebel <frieb...@amazon.de> > Reviewed-by: Julien Grall <jgr...@amazon.co.uk>
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. ~Andrew
