Re: [PATCH] pstore: Remove duplicate invoking of persistent_ram_zap()
From: Peng15 Wang 王鹏
Date: Sat Oct 27 2018 - 04:53:14 EST
>From: Kees Cook <keescook@xxxxxxxxxxxx>
>Sent: Friday, October 26, 2018 17:44
>To: Peng15 Wang 王鹏
>Cc: anton@xxxxxxxxxx; ccross@xxxxxxxxxxx; tony.luck@xxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx
>Subject: Re: [PATCH] pstore: Remove duplicate invoking of persistent_ram_zap()
>On Fri, Oct 26, 2018 at 4:41 AM, Peng15 Wang 王鹏 <wangpeng15@xxxxxxxxxx> wrote:
>> From: Peng Wang <wangpeng15@xxxxxxxxxx>
>> When initialing przs with invalid data in buffer(no PERSISTENT_RAM_SIG),
>> function call path is like this:
>> ramoops_init_prz ->
>> |-> persistent_ram_new -> persistent_ram_post_init -> persistent_ram_zap
>> |--> persistent_ram_zap
>There does appear to be a duplicate call to persistent_ram_zap() in this case.
>> As we can see, persistent_ram_zap() is called twice.
>> We can avoid this by removing it in ramoops_init_prz(),
>>and only call it in persistent_ram_post_init().
>However, I think the proposed fix doesn't work the way it should.
>There are two prz init paths: ramoops_init_prz() (a single prz) and
>ramoops_init_przs (multiple przs). The "dump" and "ftrace" cases use
>the latter. In those, there is no call to persistent_ram_zap() if the
>buffer is valid.
>In other words:
>ramoops_init_prz() unconditionally calls persistent_ram_zap(). (And
>may call it twice if there is a mismatch of the magic header.)
>ramoops_init_przs() only calls persistent_ram_zap() when the magic
>header is wrong.
>The proposed patch unconditionally zaps all regions, which means we'd
>lose "dump" and "ftrace" across the next reboot.
>Perhaps we could make it an option to persistent_ram_new()?
Thanks for your reply.
You are right, this patch does zap regions unconditionally when it comes to "dump" and
"ftrace". Sorry for the inconvenience owing to my previous mistake.
I have tried adding an option to persistent_ram_new() according to your advice and will send
a V2 version patch later. Could you please kindly pay any attention to it? Thank you!