On 01.04.2020 17:06, Robert Elz wrote: > Date: Wed, 1 Apr 2020 15:54:15 +0200 > From: Kamil Rytarowski <n...@gmx.com> > Message-ID: <969362d2-d93e-2cf4-7437-ab593ab11...@gmx.com> > > | 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? >
Going for 2. is a little bit safer and we can reduce researching corner cases that might never happen anyway. > kre >