linux-kernel - Re: [PATCH 2/2] Save current timestamp part of dmesg while writing oops message to pstore

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-Id: <bb4be206-a5c9-0101-82fa-2f344cf19a39@linux.vnet.ibm.com>
Date:   Mon, 7 Aug 2017 10:49:56 +0530
From:   Ankit Kumar <ankit@...ux.vnet.ibm.com>
To:     Kees Cook <keescook@...omium.org>
Cc:     Anton Vorontsov <anton@...msg.org>,
        Colin Cross <ccross@...roid.com>,
        Tony Luck <tony.luck@...el.com>,
        LKML <linux-kernel@...r.kernel.org>,
        "mahesh@...ux.vnet.ibm.com" <mahesh@...ux.vnet.ibm.com>,
        Hari Bathini <hbathini@...ux.vnet.ibm.com>,
        hegdevasant@...ux.vnet.ibm.com
Subject: Re: [PATCH 2/2] Save current timestamp part of dmesg while writing
 oops message to pstore

Hi Kees,



On Tuesday 23 May 2017 02:19 PM, Ankit Kumar wrote:
> Hi Kees,
>
>
>
> On Tuesday 23 May 2017 05:21 AM, Kees Cook wrote:
>> On Mon, May 22, 2017 at 3:20 AM, Ankit Kumar 
>> <ankit@...ux.vnet.ibm.com> wrote:
>>> Currently on panic or Oops, kernel saves the last few bytes from dmesg
>>> buffer to nvram. Usually kdump does capture kernel memory and provide
>>> dmesg logs as well. But in some cases where kdump fails to capture
>>> vmcore, the dmesg buffer stored in nvram/pstore turns out to be very
>>> helpful in analyzing root cause.
>>>
>>> Present code creates pstore dump file(/sys/fs/pstore/dmesg-***) 
>>> based on
>>> timestamp(retrieved from header). Current pstore code creates dump file
>>> (/sys/fs/pstore/dmesg-***) with that timestamp. Dump file can be 
>>> analyzed
>>> based on file creation time and we can make out whether dump file 
>>> has latest
>>> data or not.
>>>
>>> But when we transfer pstore dump file(/sys/fs/pstore/dmesg-***) to 
>>> other
>>> machine or collect file using some 
>>> utilities(sosreport/supportconfig) then file
>>> timestamp gets changed and hence by looking at device file 
>>> (dmesg-***) we won't
>>> be able to identify whether dump has latest data or not.
>>>
>>> Above issue can be fixed if we also have timestamp(dump creation 
>>> time) as
>>> initial few bytes while capturing dmesg buffer to pstore dump file
>>> (/sys/fs/pstore/dmesg-***).
>>>
>>>
>>> This patch enhances pstore write code to also write timestamp as 
>>> part of data.
>>>
>>> Here is sample log of dump file:(/sys/fs/pstore/dmesg-***)
>>> Oops#1 Part1 [timestamp:1494939359.590463]
>> While I understand your rationale about possibly losing file timestamp
>> information in userspace, I think this is a solvable problem on the
>> collection side. If an additional header is needed, perhaps copy the
>> dmesg files like this:
>>
>> for i in dmesg-*; do
>>      (stat --format=%y /sys/fs/pstore/$i; \
>>       cat /sys/fs/pstore/$i) > $collect_dir/$i
>> done
>
> Yes. We can handle this in userspace. But we wanted to see if we can 
> add this as part of pstore
> log itself.
>
>
>> One of the primary concerns for pstore is the stored dump size,
>
> I understand. How about adding timestamp to file name itself? 
> Something like below


How about appending time as part of file name itself. ?
Did you get time to look at above approach.
Code can be something like below piece.

~Ankit
>
> index 792a4e5..0837365 100644
> --- a/fs/pstore/inode.c
> +++ b/fs/pstore/inode.c
> @@ -349,9 +349,10 @@ int pstore_mkfile(struct dentry *root, struct 
> pstore_record *record)
>
>         switch (record->type) {
>         case PSTORE_TYPE_DMESG:
> -               scnprintf(name, sizeof(name), "dmesg-%s-%lld%s",
> +               scnprintf(name, sizeof(name), "dmesg-%s-%lld%s-%lu.%lu",
>                           record->psi->name, record->id,
> -                         record->compressed ? ".enc.z" : "");
> +                         record->compressed ? ".enc.z" : "",
> +                         record->time.tv_sec, record->time.tv_nsec / 
> 1000);
>                 break;
>         case PSTORE_TYPE_CONSOLE:
>
>
> ~Ankit