Re: CVS commit: src/lib/librumpuser

[Robert Elz <kre%munnari.OZ.AU@localhost>]

    Date:        Wed, 1 Apr 2020 15:54:15 +0200
    From:        Kamil Rytarowski <n54%gmx.com@localhost>
    Message-ID:  <969362d2-d93e-2cf4-7437-ab593ab1139a%gmx.com@localhost>

  | Ping? This still breaks.

I am still working on it.    Best I can tell at the minute is that the \0
is potentially needed (in a theoretical sense) but not by anything
operating rationally.

That is, when rump is used the strings will already always be \0 terminated
and the extra one added (the one that is off the end of the array) is never
needed, there's always an earlier one.

However, the relevant struct (that contains the string) comes from some
other process, and while if that process is running rump code, which is
what is intended to happen, all will be OK (I believe, I am not finished
checking all of that code), if it is something else, generating rump
packets, and passing them through, then we have no idea what will
be there, and the \0 termination cannot be guaranteed (and if we don't
do something, the rump process will eventually do bizarre things, that
out of the array \0 is currently preventing that possibility).

I see two reasonable paths forward here:

1. instead of adding the \0 off the end of the array, check that the
array is already \0 terminated (it should be, and always is in the ATF
test uses of rump - I ran the tests with a check in place, and it never
failed) - the \0 is always in the final byte of the array (the one you
overwrote in your earlier change, which meant that the changed line was
just a no-op, in practice, as suspected earlier.)

2. When we are reading an exec rump struct, allocate (and zero - the zero
part is already present) 1 byte more than will be received from the
sending process, so that the final byte will always remain as a \0, and
we will absolutely guarantee that the string will be \0 terminated (in
all normal cases it would end up terminated by two \0's).

If we do either of these, we don't need to waste time verifying that rump
always does send (in every case) a \0 terminated string (digging through the
code to work out where some of these structs get built is a slow process)
as the actual problem will be solved either way.

Solution 1 makes it an error, and the rup process will fail the exec if
the path isn't correctly \0 terminated.   Solution 2 does what the code
currently does (effectively) adding a \0 beyond the string that is received
from the sending process, but does it within the array bounds (by making the
array bigger) rather than outside them.

Opinions for which is better?

kre