[SLOF] [PATCH v2 20/20] make last entry in log appear

[Stefan Berger]

On 11/19/2015 12:05 PM, Thomas Huth wrote:
> On 19/11/15 17:21, Stefan Berger wrote:
>> On 11/19/2015 06:34 AM, Thomas Huth wrote:
>>> On 17/11/15 18:02, Stefan Berger wrote:
>>>> From: Stefan Berger <stefanb at linux.vnet.ibm.com>
>>>>
>>>> Due to the off-by-one error in Linux, we need to add a 1 to the
>>>> actual size of the log.
>>> If there's a bug in Linux, why don't you fix it there instead?
>>> Introducing wrong behavior here sounds wrong to me.
>> The problem is that this a bug that's been in Linux basically forever.
>> So it's unfortunately 'cheaper' now to fix it here.
> Well, I don't think so. It's maybe cheaper at a first glance, but
> including hacks like this will most likely result in much more severe
> problems in the future:
>
> - If Linux gets fixed, you suddenly would see an invalid data there at
>    the end?


No. Linux (shared) x86/ppc log display code today can deal with a buffer 
where only part of it is filled with log data, let's say 5kb out of 
64kb. It knows where the end of the log is and, since it's untrusted 
data it walks over, has to check at every step whether it walks over the 
end of the buffer and over the end of the log. I think every 
implementation retrieving such a log has to watch where it steps.

http://lxr.free-electrons.com/source/drivers/char/tpm/tpm_eventlog.c#L111

As stated, in the future the ppc64 code have an API that will return the 
log buffer size rather than the log size and will then behave in the 
same way as x86 does today.

[An improvement for this patch would be to check whether the returned 
log size is the same as the allocated buffer size and only add 1 if it's 
less than the allocated buffer size. But it see to controversy -- not 
unexpected.]


>
> - Linux is not the only OS that could be running under QEMU-sPAPR
>    machine in the future ... maybe we've got FreeBSD or something else
>    one day ... and then you don't want to mess with firmware interfaces
>    there that have been broken on purpose just for Linux.

Should be the same as with Linux. When walking over the log entries, 
check that we don't step over the end of the buffer and not over the end 
of the log.

>
> - What if PowerVM one day supports the vTPM interface (or does it
>    already?) ... should it also include this hack, or not ... and
>    then would this also be required for AIX etc. ...?

PowerVM does already support the vTPM interface. Also here AIX has to 
watch out how it walks over the untrusted log / log buffer.

>
> So better get this properly fixed where it should be fixed, i.e. in the
> Linux kernel (and you can always send your patch to the "stable" kernel
> mailing lists, too, so that also older kernel trees will pick up the fix).

I knew it would be controversial...

Regards,
     Stefan