Merge tag 'pstore-v4.13-rc1' of git://git.kernel.org/pub/scm/linux/kernel/git/kees/linux

Pull pstore updates from Kees Cook:
 "Various fixes and tweaks for the pstore subsystem.

  Highlights:

   - use memdup_user() instead of open-coded copies (Geliang Tang)

   - fix record memory leak during initialization (Douglas Anderson)

   - avoid confused compressed record warning (Ankit Kumar)

   - prepopulate record timestamp and remove redundant logic from
     backends"

* tag 'pstore-v4.13-rc1' of git://git.kernel.org/pub/scm/linux/kernel/git/kees/linux:
  powerpc/nvram: use memdup_user
  pstore: use memdup_user
  pstore: Fix format string to use %u for record id
  pstore: Populate pstore record->time field
  pstore: Create common record initializer
  efi-pstore: Refactor erase routine
  pstore: Avoid potential infinite loop
  pstore: Fix leaked pstore_record in pstore_get_backend_records()
  pstore: Don't warn if data is uncompressed and type is not PSTORE_TYPE_DMESG

arch/powerpc/kernel/nvram_64.c

@@ -792,21 +792,17 @@ static ssize_t dev_nvram_write(struct file *file, const char __user *buf,
 	count = min_t(size_t, count, size - *ppos);
 	count = min(count, PAGE_SIZE);
 
-	ret = -ENOMEM;
-	tmp = kmalloc(count, GFP_KERNEL);
-	if (!tmp)
-		goto out;
-
-	ret = -EFAULT;
-	if (copy_from_user(tmp, buf, count))
+	tmp = memdup_user(buf, count);
+	if (IS_ERR(tmp)) {
+		ret = PTR_ERR(tmp);
 		goto out;
+	}
 
 	ret = ppc_md.nvram_write(tmp, count, ppos);
 
-out:
 	kfree(tmp);
+out:
 	return ret;
-
 }
 
 static long dev_nvram_ioctl(struct file *file, unsigned int cmd,

drivers/firmware/efi/efi-pstore.c

@@ -4,7 +4,7 @@
 #include <linux/slab.h>
 #include <linux/ucs2_string.h>
 
-#define DUMP_NAME_LEN 52
+#define DUMP_NAME_LEN 66
 
 static bool efivars_pstore_disable =
 	IS_ENABLED(CONFIG_EFI_VARS_PSTORE_DEFAULT_DISABLE);
@@ -244,12 +244,12 @@ static int efi_pstore_write(struct pstore_record *record)
 	efi_guid_t vendor = LINUX_EFI_CRASH_GUID;
 	int i, ret = 0;
 
-	record->time.tv_sec = get_seconds();
-	record->time.tv_nsec = 0;
-
 	record->id = generic_id(record->time.tv_sec, record->part,
 				record->count);
 
+	/* Since we copy the entire length of name, make sure it is wiped. */
+	memset(name, 0, sizeof(name));
+
 	snprintf(name, sizeof(name), "dump-type%u-%u-%d-%lu-%c",
 		 record->type, record->part, record->count,
 		 record->time.tv_sec, record->compressed ? 'C' : 'D');
@@ -267,44 +267,20 @@ static int efi_pstore_write(struct pstore_record *record)
 	return ret;
 };
 
-struct pstore_erase_data {
-	struct pstore_record *record;
-	efi_char16_t *name;
-};
-
 /*
  * Clean up an entry with the same name
  */
 static int efi_pstore_erase_func(struct efivar_entry *entry, void *data)
 {
-	struct pstore_erase_data *ed = data;
+	efi_char16_t *efi_name = data;
 	efi_guid_t vendor = LINUX_EFI_CRASH_GUID;
-	efi_char16_t efi_name_old[DUMP_NAME_LEN];
-	efi_char16_t *efi_name = ed->name;
-	unsigned long ucs2_len = ucs2_strlen(ed->name);
-	char name_old[DUMP_NAME_LEN];
-	int i;
+	unsigned long ucs2_len = ucs2_strlen(efi_name);
 
 	if (efi_guidcmp(entry->var.VendorGuid, vendor))
 		return 0;
 
-	if (ucs2_strncmp(entry->var.VariableName,
-			  efi_name, (size_t)ucs2_len)) {
-		/*
-		 * Check if an old format, which doesn't support
-		 * holding multiple logs, remains.
-		 */
-		snprintf(name_old, sizeof(name_old), "dump-type%u-%u-%lu",
-			ed->record->type, ed->record->part,
-			ed->record->time.tv_sec);
-
-		for (i = 0; i < DUMP_NAME_LEN; i++)
-			efi_name_old[i] = name_old[i];
-
-		if (ucs2_strncmp(entry->var.VariableName, efi_name_old,
-				  ucs2_strlen(efi_name_old)))
-			return 0;
-	}
+	if (ucs2_strncmp(entry->var.VariableName, efi_name, (size_t)ucs2_len))
+		return 0;
 
 	if (entry->scanning) {
 		/*
@@ -321,35 +297,48 @@ static int efi_pstore_erase_func(struct efivar_entry *entry, void *data)
 	return 1;
 }
 
-static int efi_pstore_erase(struct pstore_record *record)
+static int efi_pstore_erase_name(const char *name)
 {
-	struct pstore_erase_data edata;
 	struct efivar_entry *entry = NULL;
-	char name[DUMP_NAME_LEN];
 	efi_char16_t efi_name[DUMP_NAME_LEN];
 	int found, i;
 
-	snprintf(name, sizeof(name), "dump-type%u-%u-%d-%lu",
-		 record->type, record->part, record->count,
-		 record->time.tv_sec);
-
-	for (i = 0; i < DUMP_NAME_LEN; i++)
+	for (i = 0; i < DUMP_NAME_LEN; i++) {
 		efi_name[i] = name[i];
-
-	edata.record = record;
-	edata.name = efi_name;
+		if (name[i] == '\0')
+			break;
+	}
 
 	if (efivar_entry_iter_begin())
 		return -EINTR;
-	found = __efivar_entry_iter(efi_pstore_erase_func, &efivar_sysfs_list, &edata, &entry);
 
-	if (found && !entry->scanning) {
-		efivar_entry_iter_end();
+	found = __efivar_entry_iter(efi_pstore_erase_func, &efivar_sysfs_list,
+				    efi_name, &entry);
+	efivar_entry_iter_end();
+
+	if (found && !entry->scanning)
 		efivar_unregister(entry);
-	} else
-		efivar_entry_iter_end();
 
-	return 0;
+	return found ? 0 : -ENOENT;
+}
+
+static int efi_pstore_erase(struct pstore_record *record)
+{
+	char name[DUMP_NAME_LEN];
+	int ret;
+
+	snprintf(name, sizeof(name), "dump-type%u-%u-%d-%lu",
+		 record->type, record->part, record->count,
+		 record->time.tv_sec);
+	ret = efi_pstore_erase_name(name);
+	if (ret != -ENOENT)
+		return ret;
+
+	snprintf(name, sizeof(name), "dump-type%u-%u-%lu",
+		record->type, record->part, record->time.tv_sec);
+	ret = efi_pstore_erase_name(name);
+
+	return ret;
 }
 
 static struct pstore_info efi_pstore_info = {

fs/pstore/inode.c

@@ -349,48 +349,48 @@ 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-%llu%s",
 			  record->psi->name, record->id,
 			  record->compressed ? ".enc.z" : "");
 		break;
 	case PSTORE_TYPE_CONSOLE:
-		scnprintf(name, sizeof(name), "console-%s-%lld",
+		scnprintf(name, sizeof(name), "console-%s-%llu",
 			  record->psi->name, record->id);
 		break;
 	case PSTORE_TYPE_FTRACE:
-		scnprintf(name, sizeof(name), "ftrace-%s-%lld",
+		scnprintf(name, sizeof(name), "ftrace-%s-%llu",
 			  record->psi->name, record->id);
 		break;
 	case PSTORE_TYPE_MCE:
-		scnprintf(name, sizeof(name), "mce-%s-%lld",
+		scnprintf(name, sizeof(name), "mce-%s-%llu",
 			  record->psi->name, record->id);
 		break;
 	case PSTORE_TYPE_PPC_RTAS:
-		scnprintf(name, sizeof(name), "rtas-%s-%lld",
+		scnprintf(name, sizeof(name), "rtas-%s-%llu",
 			  record->psi->name, record->id);
 		break;
 	case PSTORE_TYPE_PPC_OF:
-		scnprintf(name, sizeof(name), "powerpc-ofw-%s-%lld",
+		scnprintf(name, sizeof(name), "powerpc-ofw-%s-%llu",
 			  record->psi->name, record->id);
 		break;
 	case PSTORE_TYPE_PPC_COMMON:
-		scnprintf(name, sizeof(name), "powerpc-common-%s-%lld",
+		scnprintf(name, sizeof(name), "powerpc-common-%s-%llu",
 			  record->psi->name, record->id);
 		break;
 	case PSTORE_TYPE_PMSG:
-		scnprintf(name, sizeof(name), "pmsg-%s-%lld",
+		scnprintf(name, sizeof(name), "pmsg-%s-%llu",
 			  record->psi->name, record->id);
 		break;
 	case PSTORE_TYPE_PPC_OPAL:
-		scnprintf(name, sizeof(name), "powerpc-opal-%s-%lld",
+		scnprintf(name, sizeof(name), "powerpc-opal-%s-%llu",
 			  record->psi->name, record->id);
 		break;
 	case PSTORE_TYPE_UNKNOWN:
-		scnprintf(name, sizeof(name), "unknown-%s-%lld",
+		scnprintf(name, sizeof(name), "unknown-%s-%llu",
 			  record->psi->name, record->id);
 		break;
 	default:
-		scnprintf(name, sizeof(name), "type%d-%s-%lld",
+		scnprintf(name, sizeof(name), "type%d-%s-%llu",
 			  record->type, record->psi->name, record->id);
 		break;
 	}

fs/pstore/internal.h

@@ -30,5 +30,7 @@ extern void	pstore_get_backend_records(struct pstore_info *psi,
 extern int	pstore_mkfile(struct dentry *root,
 			      struct pstore_record *record);
 extern bool	pstore_is_mounted(void);
+extern void	pstore_record_init(struct pstore_record *record,
+				   struct pstore_info *psi);
 
 #endif

fs/pstore/platform.c

@@ -474,6 +474,20 @@ static size_t copy_kmsg_to_buffer(int hsize, size_t len)
 	return total_len;
 }
 
+void pstore_record_init(struct pstore_record *record,
+			struct pstore_info *psinfo)
+{
+	memset(record, 0, sizeof(*record));
+
+	record->psi = psinfo;
+
+	/* Report zeroed timestamp if called before timekeeping has resumed. */
+	if (__getnstimeofday(&record->time)) {
+		record->time.tv_sec = 0;
+		record->time.tv_nsec = 0;
+	}
+}
+
 /*
  * callback from kmsg_dump. (s2,l2) has the most recently
  * written bytes, older bytes are in (s1,l1). Save as much
@@ -509,15 +523,14 @@ static void pstore_dump(struct kmsg_dumper *dumper,
 		int header_size;
 		int zipped_len = -1;
 		size_t dump_size;
-		struct pstore_record record = {
-			.type = PSTORE_TYPE_DMESG,
-			.count = oopscount,
-			.reason = reason,
-			.part = part,
-			.compressed = false,
-			.buf = psinfo->buf,
-			.psi = psinfo,
-		};
+		struct pstore_record record;
+
+		pstore_record_init(&record, psinfo);
+		record.type = PSTORE_TYPE_DMESG;
+		record.count = oopscount;
+		record.reason = reason;
+		record.part = part;
+		record.buf = psinfo->buf;
 
 		if (big_oops_buf && is_locked) {
 			dst = big_oops_buf;
@@ -587,12 +600,12 @@ static void pstore_console_write(struct console *con, const char *s, unsigned c)
 	const char *e = s + c;
 
 	while (s < e) {
-		struct pstore_record record = {
-			.type = PSTORE_TYPE_CONSOLE,
-			.psi = psinfo,
-		};
+		struct pstore_record record;
 		unsigned long flags;
 
+		pstore_record_init(&record, psinfo);
+		record.type = PSTORE_TYPE_CONSOLE;
+
 		if (c > psinfo->bufsize)
 			c = psinfo->bufsize;
 
@@ -640,19 +653,16 @@ static int pstore_write_user_compat(struct pstore_record *record,
 	if (record->buf)
 		return -EINVAL;
 
-	record->buf = kmalloc(record->size, GFP_KERNEL);
-	if (!record->buf)
-		return -ENOMEM;
-
-	if (unlikely(copy_from_user(record->buf, buf, record->size))) {
-		ret = -EFAULT;
+	record->buf = memdup_user(buf, record->size);
+	if (unlikely(IS_ERR(record->buf))) {
+		ret = PTR_ERR(record->buf);
 		goto out;
 	}
 
 	ret = record->psi->write(record);
 
-out:
 	kfree(record->buf);
+out:
 	record->buf = NULL;
 
 	return unlikely(ret < 0) ? ret : record->size;
@@ -770,8 +780,11 @@ static void decompress_record(struct pstore_record *record)
 	int unzipped_len;
 	char *decompressed;
 
+	if (!record->compressed)
+		return;
+
 	/* Only PSTORE_TYPE_DMESG support compression. */
-	if (!record->compressed || record->type != PSTORE_TYPE_DMESG) {
+	if (record->type != PSTORE_TYPE_DMESG) {
 		pr_warn("ignored compressed record type %d\n", record->type);
 		return;
 	}
@@ -819,6 +832,7 @@ void pstore_get_backend_records(struct pstore_info *psi,
 				struct dentry *root, int quiet)
 {
 	int failed = 0;
+	unsigned int stop_loop = 65536;
 
 	if (!psi || !root)
 		return;
@@ -832,7 +846,7 @@ void pstore_get_backend_records(struct pstore_info *psi,
 	 * may reallocate record.buf. On success, pstore_mkfile() will keep
 	 * the record.buf, so free it only on failure.
 	 */
-	for (;;) {
+	for (; stop_loop; stop_loop--) {
 		struct pstore_record *record;
 		int rc;
 
@@ -841,13 +855,15 @@ void pstore_get_backend_records(struct pstore_info *psi,
 			pr_err("out of memory creating record\n");
 			break;
 		}
-		record->psi = psi;
+		pstore_record_init(record, psi);
 
 		record->size = psi->read(record);
 
 		/* No more records left in backend? */
-		if (record->size <= 0)
+		if (record->size <= 0) {
+			kfree(record);
 			break;
+		}
 
 		decompress_record(record);
 		rc = pstore_mkfile(root, record);
@@ -865,8 +881,11 @@ void pstore_get_backend_records(struct pstore_info *psi,
 	mutex_unlock(&psi->read_mutex);
 
 	if (failed)
-		pr_warn("failed to load %d record(s) from '%s'\n",
+		pr_warn("failed to create %d record(s) from '%s'\n",
 			failed, psi->name);
+	if (!stop_loop)
+		pr_err("looping? Too many records seen from '%s'\n",
+			psi->name);
 }
 
 static void pstore_dowork(struct work_struct *work)

fs/pstore/pmsg.c

@@ -22,16 +22,16 @@ static DEFINE_MUTEX(pmsg_lock);
 static ssize_t write_pmsg(struct file *file, const char __user *buf,
 			  size_t count, loff_t *ppos)
 {
-	struct pstore_record record = {
-		.type = PSTORE_TYPE_PMSG,
-		.size = count,
-		.psi = psinfo,
-	};
+	struct pstore_record record;
 	int ret;
 
 	if (!count)
 		return 0;
 
+	pstore_record_init(&record, psinfo);
+	record.type = PSTORE_TYPE_PMSG;
+	record.size = count;
+
 	/* check outside lock, page in any data. write_user also checks */
 	if (!access_ok(VERIFY_READ, buf, count))
 		return -EFAULT;

fs/pstore/ram.c

@@ -27,7 +27,6 @@
 #include <linux/module.h>
 #include <linux/version.h>
 #include <linux/pstore.h>
-#include <linux/time.h>
 #include <linux/io.h>
 #include <linux/ioport.h>
 #include <linux/platform_device.h>
@@ -356,20 +355,15 @@ static ssize_t ramoops_pstore_read(struct pstore_record *record)
 }
 
 static size_t ramoops_write_kmsg_hdr(struct persistent_ram_zone *prz,
-				     bool compressed)
+				     struct pstore_record *record)
 {
 	char *hdr;
-	struct timespec timestamp;
 	size_t len;
 
-	/* Report zeroed timestamp if called before timekeeping has resumed. */
-	if (__getnstimeofday(&timestamp)) {
-		timestamp.tv_sec = 0;
-		timestamp.tv_nsec = 0;
-	}
 	hdr = kasprintf(GFP_ATOMIC, RAMOOPS_KERNMSG_HDR "%lu.%lu-%c\n",
-		(long)timestamp.tv_sec, (long)(timestamp.tv_nsec / 1000),
-		compressed ? 'C' : 'D');
+		record->time.tv_sec,
+		record->time.tv_nsec / 1000,
+		record->compressed ? 'C' : 'D');
 	WARN_ON_ONCE(!hdr);
 	len = hdr ? strlen(hdr) : 0;
 	persistent_ram_write(prz, hdr, len);
@@ -440,7 +434,7 @@ static int notrace ramoops_pstore_write(struct pstore_record *record)
 	prz = cxt->dprzs[cxt->dump_write_cnt];
 
 	/* Build header and append record contents. */
-	hlen = ramoops_write_kmsg_hdr(prz, record->compressed);
+	hlen = ramoops_write_kmsg_hdr(prz, record);
 	size = record->size;
 	if (size + hlen > prz->buffer_size)
 		size = prz->buffer_size - hlen;

include/linux/pstore.h

@@ -138,7 +138,10 @@ struct pstore_record {
  *		memory allocation may be broken during an Oops. Regardless,
  *		@buf must be proccesed or copied before returning. The
  *		backend is also expected to write @id with something that
- 8		can help identify this record to a future @erase callback.
+ *		can help identify this record to a future @erase callback.
+ *		The @time field will be prepopulated with the current time,
+ *		when available. The @size field will have the size of data
+ *		in @buf.
  *
  *	Returns 0 on success, and non-zero on error.
  *