Problem with 'test-pci'

[Norman Feske]

Hi Frank,

Frank Kaiser wrote:
> I prefer to fix the root cause. However my attempt outlined below did
> not work, since it does not take into account that the function writes a
> ‘\0’ at the end of the destination string (something the standard C
> library function doesn’t do), for which the calculated /size/ value has
> to be adjusted. The final fix of /Genode::strncpy()/ is:

Indeed. The libc version gives no indication of whether the string was
cut or not. So you would need to check dst[size - 1] == 0 for the
zero padding (which our version does not implement). So we decided to
ensure that the result of the function is always a properly terminated
string.

The strncpy function is only a part of a bigger problem, which is the
reason why we deferred the fix until now. The root of the problem is
that the end of a data spaces acquired from core's ROM service cannot be
expected to be padded with zeros. In the corner case of a data module
with the exact size of 4096 bytes, there is no padding at all. However,
we mistakenly specified the local address of the mapped dataspace
directly to the Xml_node constructor for parsing the config file. The
constructor, however, expected a null-terminated string. Our fix
introduces a further constructor argument for specifying the maximum
length of the string. The proper handling of respecting this boundary
needed code changes in the XML parser, the tokenizer, and some string
functions (e.g., ascii_to_ulong). The strncpy function is final element
in the chain of troublemakers ;-)

Looking from the implementation viewpoint, the strncpy function actually
complied to the function interface and was not buggy. The interface
expects a string as argument 'src', which is, by definition, null-
terminated. The size argument is normally used to specify the boundary
of the 'dst' buffer, which worked correctly. The problem is that strncpy
is called with an invalid 'src' argument and the implicit assumption
that the function will not touch memory beyond 'src + size - 1'.
However, we need this semantics for our particular data-space-parsing
use-case.

I have checked in the complete fix into our SVN. For strncpy, I went for
a single loop rather than two loops (checking the size and memcpy) and I
think that the resulting code is more obvious. In the process, I also
complemented the documentation with regard to the differences between
our implementation and the libc version.

Best regards
Norman