hi Kees Cook,

On 2020/5/12 PM 1:15, Kees Cook wrote:

On Tue, May 12, 2020 at 11:55:20AM +0800, WeiXiong Liao wrote:

quoted

On 2020/5/12 AM 7:32, Kees Cook wrote:

quoted

[...]
+struct psz_context {
+	struct pstore_zone **kpszs;
+	unsigned int kmsg_max_cnt;
+	unsigned int kmsg_read_cnt;
+	unsigned int kmsg_write_cnt;
+	/*
+	 * These counters should be calculated during recovery.
+	 * It records the oops/panic times after crashes rather than boots.
+	 */
+	unsigned int oops_counter;
+	unsigned int panic_counter;

oops/panic_counter is designed to count the crash times since the
linux kernel was installed. pstore/zone lookup the max counter from all
valid kmsg zones when recovery and saves them to oops/panic_counter.
However, they are unable to get real number if we remove files. It's
not serious, we can fix it after this series.

Since the kernel was installed? I don't see a kernel version check in
here? Or do you mean "since ever", in that it's a rolling count?

Yes, "since ever".

quoted

And since pstore supports "max_reason", should pstore/zone count for
other reason?

For now, no. I opted to try to keep this as simple as possible a port
from dump_oops to max_reason for now.

OK.

quoted

quoted

+static inline int psz_kmsg_erase(struct psz_context *cxt,
+		struct pstore_zone *zone, struct pstore_record *record)
+{
+	struct psz_buffer *buffer = zone->buffer;
+	struct psz_kmsg_header *hdr =
+		(struct psz_kmsg_header *)buffer->data;
+
+	if (unlikely(!psz_ok(zone)))
+		return 0;
+	/* this zone is already updated, no need to erase */
+	if (record->count != hdr->counter)
+		return 0;

These codes is to fix bug that user remove files on pstore filesystem
but kmsg zone is already updated and pstore/zone should not erase
zone. It work for oops and panic because the count number is increasing.
However, it's useless for other reason of kmsg. We can fix it after this
series.

Okay, sounds good.

-- 
WeiXiong Liao