* [PATCH] pstore/ram: Fix console ramoops to show the previous boot logs
@ 2019-01-17 6:26 Sai Prakash Ranjan
2019-01-17 15:18 ` Joel Fernandes
2019-01-17 17:11 ` Kees Cook
0 siblings, 2 replies; 4+ messages in thread
From: Sai Prakash Ranjan @ 2019-01-17 6:26 UTC (permalink / raw)
To: Kees Cook, Joel Fernandes, Anton Vorontsov, Colin Cross,
Tony Luck, Guenter Roeck
Cc: Rajendra Nayak, Vivek Gautam, Sibi Sankar, Stephen Boyd,
Doug Anderson, linux-arm-kernel, linux-kernel, linux-arm-msm,
Sai Prakash Ranjan
commit b05c950698fe ("pstore/ram: Simplify ramoops_get_next_prz()
arguments") changed update assignment in getting next persistent
ram zone by adding a check for record type. But the check always
returns true since the record type is assigned 0. And this breaks
console ramoops by showing current console log instead of previous
log on warm reset and hard reset (actually hard reset should not
be showing any logs).
Fix this by having persistent ram zone type check instead of record
type check. Tested this on SDM845 MTP and dragonboard 410c.
Reproducing this issue is simple as below:
1. Trigger hard reset and mount pstore. Will see console-ramoops
record in the mounted location which is the current log.
2. Trigger warm reset and mount pstore. Will see the current
console-ramoops record instead of previous record.
Fixes: b05c950698fe ("pstore/ram: Simplify ramoops_get_next_prz() arguments")
Signed-off-by: Sai Prakash Ranjan <saiprakash.ranjan@codeaurora.org>
---
I guess commit msg can be improved.
---
fs/pstore/ram.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/fs/pstore/ram.c b/fs/pstore/ram.c
index 96f7d32cd184..441a44215456 100644
--- a/fs/pstore/ram.c
+++ b/fs/pstore/ram.c
@@ -128,7 +128,7 @@ ramoops_get_next_prz(struct persistent_ram_zone *przs[], int id,
struct pstore_record *record)
{
struct persistent_ram_zone *prz;
- bool update = (record->type == PSTORE_TYPE_DMESG);
+ bool update;
/* Give up if we never existed or have hit the end. */
if (!przs)
@@ -138,6 +138,8 @@ ramoops_get_next_prz(struct persistent_ram_zone *przs[], int id,
if (!prz)
return NULL;
+ update = (prz->type == PSTORE_TYPE_DMESG);
+
/* Update old/shadowed buffer. */
if (update)
persistent_ram_save_old(prz);
--
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation
^ permalink raw reply related [flat|nested] 4+ messages in thread* Re: [PATCH] pstore/ram: Fix console ramoops to show the previous boot logs
2019-01-17 6:26 [PATCH] pstore/ram: Fix console ramoops to show the previous boot logs Sai Prakash Ranjan
@ 2019-01-17 15:18 ` Joel Fernandes
2019-01-17 15:28 ` Sai Prakash Ranjan
2019-01-17 17:11 ` Kees Cook
1 sibling, 1 reply; 4+ messages in thread
From: Joel Fernandes @ 2019-01-17 15:18 UTC (permalink / raw)
To: Sai Prakash Ranjan
Cc: Kees Cook, Anton Vorontsov, Colin Cross, Tony Luck,
Guenter Roeck, Rajendra Nayak, Vivek Gautam, Sibi Sankar,
Stephen Boyd, Doug Anderson, linux-arm-kernel, linux-kernel,
linux-arm-msm
On Thu, Jan 17, 2019 at 11:56:20AM +0530, Sai Prakash Ranjan wrote:
> commit b05c950698fe ("pstore/ram: Simplify ramoops_get_next_prz()
> arguments") changed update assignment in getting next persistent
> ram zone by adding a check for record type. But the check always
> returns true since the record type is assigned 0. And this breaks
> console ramoops by showing current console log instead of previous
> log on warm reset and hard reset (actually hard reset should not
> be showing any logs).
>
> Fix this by having persistent ram zone type check instead of record
> type check. Tested this on SDM845 MTP and dragonboard 410c.
>
> Reproducing this issue is simple as below:
>
> 1. Trigger hard reset and mount pstore. Will see console-ramoops
> record in the mounted location which is the current log.
>
> 2. Trigger warm reset and mount pstore. Will see the current
> console-ramoops record instead of previous record.
>
> Fixes: b05c950698fe ("pstore/ram: Simplify ramoops_get_next_prz() arguments")
> Signed-off-by: Sai Prakash Ranjan <saiprakash.ranjan@codeaurora.org>
Acked-by: Joel Fernandes (Google) <joel@joelfernandes.org>
thanks!
- Joel
>
> ---
>
> I guess commit msg can be improved.
> ---
> fs/pstore/ram.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/fs/pstore/ram.c b/fs/pstore/ram.c
> index 96f7d32cd184..441a44215456 100644
> --- a/fs/pstore/ram.c
> +++ b/fs/pstore/ram.c
> @@ -128,7 +128,7 @@ ramoops_get_next_prz(struct persistent_ram_zone *przs[], int id,
> struct pstore_record *record)
> {
> struct persistent_ram_zone *prz;
> - bool update = (record->type == PSTORE_TYPE_DMESG);
> + bool update;
>
> /* Give up if we never existed or have hit the end. */
> if (!przs)
> @@ -138,6 +138,8 @@ ramoops_get_next_prz(struct persistent_ram_zone *przs[], int id,
> if (!prz)
> return NULL;
>
> + update = (prz->type == PSTORE_TYPE_DMESG);
> +
> /* Update old/shadowed buffer. */
> if (update)
> persistent_ram_save_old(prz);
> --
> QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
> of Code Aurora Forum, hosted by The Linux Foundation
>
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH] pstore/ram: Fix console ramoops to show the previous boot logs
2019-01-17 15:18 ` Joel Fernandes
@ 2019-01-17 15:28 ` Sai Prakash Ranjan
0 siblings, 0 replies; 4+ messages in thread
From: Sai Prakash Ranjan @ 2019-01-17 15:28 UTC (permalink / raw)
To: Joel Fernandes
Cc: Kees Cook, Anton Vorontsov, Colin Cross, Tony Luck,
Guenter Roeck, Rajendra Nayak, Vivek Gautam, Sibi Sankar,
Stephen Boyd, Doug Anderson, linux-arm-kernel, linux-kernel,
linux-arm-msm
On 1/17/2019 8:48 PM, Joel Fernandes wrote:
> On Thu, Jan 17, 2019 at 11:56:20AM +0530, Sai Prakash Ranjan wrote:
>> commit b05c950698fe ("pstore/ram: Simplify ramoops_get_next_prz()
>> arguments") changed update assignment in getting next persistent
>> ram zone by adding a check for record type. But the check always
>> returns true since the record type is assigned 0. And this breaks
>> console ramoops by showing current console log instead of previous
>> log on warm reset and hard reset (actually hard reset should not
>> be showing any logs).
>>
>> Fix this by having persistent ram zone type check instead of record
>> type check. Tested this on SDM845 MTP and dragonboard 410c.
>>
>> Reproducing this issue is simple as below:
>>
>> 1. Trigger hard reset and mount pstore. Will see console-ramoops
>> record in the mounted location which is the current log.
>>
>> 2. Trigger warm reset and mount pstore. Will see the current
>> console-ramoops record instead of previous record.
>>
>> Fixes: b05c950698fe ("pstore/ram: Simplify ramoops_get_next_prz() arguments")
>> Signed-off-by: Sai Prakash Ranjan <saiprakash.ranjan@codeaurora.org>
>
>
> Acked-by: Joel Fernandes (Google) <joel@joelfernandes.org>
>
Thanks Joel.
--
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] pstore/ram: Fix console ramoops to show the previous boot logs
2019-01-17 6:26 [PATCH] pstore/ram: Fix console ramoops to show the previous boot logs Sai Prakash Ranjan
2019-01-17 15:18 ` Joel Fernandes
@ 2019-01-17 17:11 ` Kees Cook
1 sibling, 0 replies; 4+ messages in thread
From: Kees Cook @ 2019-01-17 17:11 UTC (permalink / raw)
To: Sai Prakash Ranjan
Cc: Joel Fernandes, Anton Vorontsov, Colin Cross, Tony Luck,
Guenter Roeck, Rajendra Nayak, Vivek Gautam, Sibi Sankar,
Stephen Boyd, Doug Anderson, linux-arm-kernel, LKML,
linux-arm-msm
On Wed, Jan 16, 2019 at 10:27 PM Sai Prakash Ranjan
<saiprakash.ranjan@codeaurora.org> wrote:
>
> commit b05c950698fe ("pstore/ram: Simplify ramoops_get_next_prz()
> arguments") changed update assignment in getting next persistent
> ram zone by adding a check for record type. But the check always
> returns true since the record type is assigned 0. And this breaks
> console ramoops by showing current console log instead of previous
> log on warm reset and hard reset (actually hard reset should not
> be showing any logs).
>
> Fix this by having persistent ram zone type check instead of record
> type check. Tested this on SDM845 MTP and dragonboard 410c.
>
> Reproducing this issue is simple as below:
>
> 1. Trigger hard reset and mount pstore. Will see console-ramoops
> record in the mounted location which is the current log.
>
> 2. Trigger warm reset and mount pstore. Will see the current
> console-ramoops record instead of previous record.
>
> Fixes: b05c950698fe ("pstore/ram: Simplify ramoops_get_next_prz() arguments")
> Signed-off-by: Sai Prakash Ranjan <saiprakash.ranjan@codeaurora.org>
Whoops! Thanks for catching this! I've adjusted the patch slightly (no
need for a local variable), and will send this to Linus.
-Kees
>
> ---
>
> I guess commit msg can be improved.
> ---
> fs/pstore/ram.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/fs/pstore/ram.c b/fs/pstore/ram.c
> index 96f7d32cd184..441a44215456 100644
> --- a/fs/pstore/ram.c
> +++ b/fs/pstore/ram.c
> @@ -128,7 +128,7 @@ ramoops_get_next_prz(struct persistent_ram_zone *przs[], int id,
> struct pstore_record *record)
> {
> struct persistent_ram_zone *prz;
> - bool update = (record->type == PSTORE_TYPE_DMESG);
> + bool update;
>
> /* Give up if we never existed or have hit the end. */
> if (!przs)
> @@ -138,6 +138,8 @@ ramoops_get_next_prz(struct persistent_ram_zone *przs[], int id,
> if (!prz)
> return NULL;
>
> + update = (prz->type == PSTORE_TYPE_DMESG);
> +
> /* Update old/shadowed buffer. */
> if (update)
> persistent_ram_save_old(prz);
> --
> QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
> of Code Aurora Forum, hosted by The Linux Foundation
>
--
Kees Cook
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2019-01-17 17:11 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-01-17 6:26 [PATCH] pstore/ram: Fix console ramoops to show the previous boot logs Sai Prakash Ranjan
2019-01-17 15:18 ` Joel Fernandes
2019-01-17 15:28 ` Sai Prakash Ranjan
2019-01-17 17:11 ` Kees Cook
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).