Source-Changes-D archive

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

Re: CVS commit: src/lib/librumpuser



Ping? We are blocked by this in GSoC now.

On 01.04.2020 20:19, Kamil Rytarowski wrote:
> On 01.04.2020 17:06, Robert Elz wrote:
>>     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?
>>
> 
> Going for 2. is a little bit safer and we can reduce researching corner
> cases that might never happen anyway.
> 
>> kre
>>
> 


Attachment: signature.asc
Description: OpenPGP digital signature



Home | Main Index | Thread Index | Old Index