On 03.06.19 16:39, Stefan Liebler wrote:
> On 6/3/19 12:45 PM, David Hildenbrand wrote:
>> On 03.06.19 12:38, Stefan Liebler wrote:
>>> On 5/31/19 4:56 PM, David Hildenbrand wrote:
>>>> The PoP (z14, 7-382) says:
>>>> Doublewords to the right of the doubleword in which the
>>>> highest-numbered facility bit is assigned for a model
>>>> may or may not be stored.
>>>>
>>>> However, stack protection in certain binaries can't deal with that.
>>>> "gzip" example code:
>>>>
>>>> f1b4: a7 08 00 03 lhi %r0,3
>>>> f1b8: b2 b0 f0 a0 stfle 160(%r15)
>>>> f1bc: e3 20 f0 b2 00 90 llgc %r2,178(%r15)
>>>> f1c2: c0 2b 00 00 00 01 nilf %r2,1
>>>> f1c8: b2 4f 00 10 ear %r1,%a0
>>>> f1cc: b9 14 00 22 lgfr %r2,%r2
>>>> f1d0: eb 11 00 20 00 0d sllg %r1,%r1,32
>>>> f1d6: b2 4f 00 11 ear %r1,%a1
>>>> f1da: d5 07 f0 b8 10 28 clc 184(8,%r15),40(%r1)
>>>> f1e0: a7 74 00 06 jne f1ec <file_read@@Base+0x1bc>
>>>> f1e4: eb ef f1 30 00 04 lmg %r14,%r15,304(%r15)
>>>> f1ea: 07 fe br %r14
>>>> f1ec: c0 e5 ff ff 9d 6e brasl %r14,2cc8
>>>> <__stack_chk_fail@plt>
>>>>
>>>> In QEMU, we currently have:
>>>> max_bytes = 24
>>>> the code asks for (3 + 1) doublewords == 32 bytes.
>>>>
>>>> If we write 32 bytes instead of only 24, and return "2 + 1" doublewords
>>>> ("one less than the number of doulewords needed to contain all of the
>>>> facility bits"), the example code detects a stack corruption.
>>>>
>>>> In my opinion, the code is wrong. However, it seems to work fine on
>>>> real machines. So let's limit storing to the minimum of the requested
>>>> and the maximum doublewords.
>>> Hi David,
>>>
>>> Thanks for catching this. I've reported the "gzip" example to Ilya and
>>> indeed, r0 is setup too large. He will fix it in gzip.
>>>
>>> You've mentioned, that this is detected in certain binaries.
>>> Can you please share those occurrences.
>>
>> Hi Stafan,
>>
>> thanks for your reply.
>>
>> I didn't track all occurrences, it *could* be that it was only gzip in
>> the background making other processes fail.
>>
>> For example, the systemd "vitual console setup" unit failed, too, which
>> was fixed by this change.
> At least "objdump -d /usr/lib/systemd/systemd-vconsole-setup" does not
> contain the stfle instruction, but "ldd
> /usr/lib/systemd/systemd-vconsole-setup" is showing libz.so which could
> also contain Ilya's patches with the stfle instruction (I assume there
> is the same bug as in gzip). But I have no idea if libz is really called.
>>
>> I also remember, seeing segfaults in rpmbuild, for example. They only
>> "changed" with this fix - I m still chasing different errors. :)
> The same applies for rpmbuild.
>>
>> You mentioned "He will fix it in gzip", so I assume this is a gzip issue
>> and not a gcc/glibc/whatever toolchain issue?
>>
> Yes, this is a gzip bug. r0 was initialized with:
> (sizeof(array-on-stack) / 8)
> instead of:
> (sizeof(array-on-stack) / 8) - 1
>
> Ilya will fix it in gzip and zlib.
> @Ilya: Please correct me if I'm wrong.
>
Makes sense, thanks for the explanation!
--
Thanks,
David / dhildenb
