Re: [PATCH v2] pstore/ram: no timekeeping calls when unavailable

[Kees Cook]

On Fri, Nov 9, 2012 at 4:56 PM, John Stultz <johnstul@xxxxxxxxxx> wrote:
> On 11/05/2012 02:00 PM, Kees Cook wrote:
>>
>> We must not call timekeeping functions unless they are available. If we
>> dump
>> before they have resumed, avoid a WARN_ON by setting the timestamp to 0.
>>
>> Since the "ram" pstore driver can be a module, we must have
>> timekeeping_suspended exported.
>>
>> Reported-by: Doug Anderson <dianders@xxxxxxxxxxxx>
>> Cc: Anton Vorontsov <cbouatmailru@xxxxxxxxx>
>> Cc: John Stultz <johnstul@xxxxxxxxxx>
>> Signed-off-by: Kees Cook <keescook@xxxxxxxxxxxx>
>> ---
>> v2:
>>   - export needed for timekeeping_suspended (thanks to Fengguang Wu).
>> ---
>>   fs/pstore/ram.c           |    8 +++++++-
>>   kernel/time/timekeeping.c |    1 +
>>   2 files changed, 8 insertions(+), 1 deletion(-)
>>
>> diff --git a/fs/pstore/ram.c b/fs/pstore/ram.c
>> index 1a4f6da..6d014e0 100644
>> --- a/fs/pstore/ram.c
>> +++ b/fs/pstore/ram.c
>> @@ -171,7 +171,13 @@ static size_t ramoops_write_kmsg_hdr(struct
>> persistent_ram_zone *prz)
>>         struct timeval timestamp;
>>         size_t len;
>>
>> -       do_gettimeofday(&timestamp);
>> +       /* Handle dumping before timekeeping has resumed. */
>> +       if (unlikely(timekeeping_suspended)) {
>> +               timestamp.tv_sec = 0;
>> +               timestamp.tv_usec = 0;
>> +       } else
>> +               do_gettimeofday(&timestamp);
>> +
>
> Would nulling out the timestamp be better done in do_gettimeofday()?  That
> way we don't have to export timekeeping internals and users would get
> something more sane for this corner case.

Well... I'm not sure. If we don't want to expose the
timekeeping_suspended variable, maybe we need a function to check
this? I think it's probably better to find the users of timekeeping
that could call it when suspended. That's why I figured the BUG was
there. Very very few things should be attempting to call gettimeofday
in a place where it might be suspended. As such, it seems like those
things should be able to determine how to handle it. Maybe not
everything would be sensible to get back 0s.

In this particular case, I'm fine with removing the BUG and returning
0 instead, since that's fine for ramoops. :)

-Kees

-- 
Kees Cook
Chrome OS Security
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/