LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
* [PATCH v2] pstore: Convert internal records to timespec64
@ 2018-05-14 22:50 Kees Cook
  2018-05-15 17:42 ` Deepa Dinamani
  2018-05-30 15:06 ` Arnd Bergmann
  0 siblings, 2 replies; 3+ messages in thread
From: Kees Cook @ 2018-05-14 22:50 UTC (permalink / raw)
  To: Deepa Dinamani; +Cc: linux-kernel, Anton Vorontsov, Colin Cross, Tony Luck

This prepares pstore for converting the VFS layer to timespec64.

Signed-off-by: Kees Cook <keescook@chromium.org>
---
I can carry this for v4.18 unless you'd like to do it as part of your series?
---
 drivers/firmware/efi/efi-pstore.c | 27 ++++++++++++++-------------
 fs/pstore/inode.c                 |  3 ++-
 fs/pstore/platform.c              |  2 +-
 fs/pstore/ram.c                   | 21 ++++++++++++++-------
 include/linux/pstore.h            |  2 +-
 5 files changed, 32 insertions(+), 23 deletions(-)

diff --git a/drivers/firmware/efi/efi-pstore.c b/drivers/firmware/efi/efi-pstore.c
index 5a0fa939d70f..cfe87b465819 100644
--- a/drivers/firmware/efi/efi-pstore.c
+++ b/drivers/firmware/efi/efi-pstore.c
@@ -28,10 +28,9 @@ static int efi_pstore_close(struct pstore_info *psi)
 	return 0;
 }
 
-static inline u64 generic_id(unsigned long timestamp,
-			     unsigned int part, int count)
+static inline u64 generic_id(u64 timestamp, unsigned int part, int count)
 {
-	return ((u64) timestamp * 100 + part) * 1000 + count;
+	return (timestamp * 100 + part) * 1000 + count;
 }
 
 static int efi_pstore_read_func(struct efivar_entry *entry,
@@ -42,7 +41,8 @@ static int efi_pstore_read_func(struct efivar_entry *entry,
 	int i;
 	int cnt;
 	unsigned int part;
-	unsigned long time, size;
+	unsigned long size;
+	u64 time;
 
 	if (efi_guidcmp(entry->var.VendorGuid, vendor))
 		return 0;
@@ -50,7 +50,7 @@ static int efi_pstore_read_func(struct efivar_entry *entry,
 	for (i = 0; i < DUMP_NAME_LEN; i++)
 		name[i] = entry->var.VariableName[i];
 
-	if (sscanf(name, "dump-type%u-%u-%d-%lu-%c",
+	if (sscanf(name, "dump-type%u-%u-%d-%llu-%c",
 		   &record->type, &part, &cnt, &time, &data_type) == 5) {
 		record->id = generic_id(time, part, cnt);
 		record->part = part;
@@ -62,7 +62,7 @@ static int efi_pstore_read_func(struct efivar_entry *entry,
 		else
 			record->compressed = false;
 		record->ecc_notice_size = 0;
-	} else if (sscanf(name, "dump-type%u-%u-%d-%lu",
+	} else if (sscanf(name, "dump-type%u-%u-%d-%llu",
 		   &record->type, &part, &cnt, &time) == 4) {
 		record->id = generic_id(time, part, cnt);
 		record->part = part;
@@ -71,7 +71,7 @@ static int efi_pstore_read_func(struct efivar_entry *entry,
 		record->time.tv_nsec = 0;
 		record->compressed = false;
 		record->ecc_notice_size = 0;
-	} else if (sscanf(name, "dump-type%u-%u-%lu",
+	} else if (sscanf(name, "dump-type%u-%u-%llu",
 			  &record->type, &part, &time) == 3) {
 		/*
 		 * Check if an old format,
@@ -250,9 +250,10 @@ static int efi_pstore_write(struct pstore_record *record)
 	/* 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",
+	snprintf(name, sizeof(name), "dump-type%u-%u-%d-%lld-%c",
 		 record->type, record->part, record->count,
-		 record->time.tv_sec, record->compressed ? 'C' : 'D');
+		 (long long)record->time.tv_sec,
+		 record->compressed ? 'C' : 'D');
 
 	for (i = 0; i < DUMP_NAME_LEN; i++)
 		efi_name[i] = name[i];
@@ -327,15 +328,15 @@ 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",
+	snprintf(name, sizeof(name), "dump-type%u-%u-%d-%lld",
 		 record->type, record->part, record->count,
-		 record->time.tv_sec);
+		 (long long)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);
+	snprintf(name, sizeof(name), "dump-type%u-%u-%lld",
+		record->type, record->part, (long long)record->time.tv_sec);
 	ret = efi_pstore_erase_name(name);
 
 	return ret;
diff --git a/fs/pstore/inode.c b/fs/pstore/inode.c
index 5fcb845b9fec..75afe5eb0574 100644
--- a/fs/pstore/inode.c
+++ b/fs/pstore/inode.c
@@ -392,7 +392,8 @@ int pstore_mkfile(struct dentry *root, struct pstore_record *record)
 	inode->i_private = private;
 
 	if (record->time.tv_sec)
-		inode->i_mtime = inode->i_ctime = record->time;
+		inode->i_mtime = inode->i_ctime =
+			timespec64_to_timespec(record->time);
 
 	d_add(dentry, inode);
 
diff --git a/fs/pstore/platform.c b/fs/pstore/platform.c
index dc720573fd53..c238ab8ba31d 100644
--- a/fs/pstore/platform.c
+++ b/fs/pstore/platform.c
@@ -328,7 +328,7 @@ void pstore_record_init(struct pstore_record *record,
 	record->psi = psinfo;
 
 	/* Report zeroed timestamp if called before timekeeping has resumed. */
-	record->time = ns_to_timespec(ktime_get_real_fast_ns());
+	record->time = ns_to_timespec64(ktime_get_real_fast_ns());
 }
 
 /*
diff --git a/fs/pstore/ram.c b/fs/pstore/ram.c
index 49b2bc114868..69e893076ab7 100644
--- a/fs/pstore/ram.c
+++ b/fs/pstore/ram.c
@@ -38,6 +38,11 @@
 
 #define RAMOOPS_KERNMSG_HDR "===="
 #define MIN_MEM_SIZE 4096UL
+#if __BITS_PER_LONG == 64
+# define TVSEC_FMT "%ld"
+#else
+# define TVSEC_FMT "%lld"
+#endif
 
 static ulong record_size = MIN_MEM_SIZE;
 module_param(record_size, ulong, 0400);
@@ -153,21 +158,23 @@ ramoops_get_next_prz(struct persistent_ram_zone *przs[], uint *c, uint max,
 	return prz;
 }
 
-static int ramoops_read_kmsg_hdr(char *buffer, struct timespec *time,
+static int ramoops_read_kmsg_hdr(char *buffer, struct timespec64 *time,
 				  bool *compressed)
 {
 	char data_type;
 	int header_length = 0;
 
-	if (sscanf(buffer, RAMOOPS_KERNMSG_HDR "%lu.%lu-%c\n%n", &time->tv_sec,
-			&time->tv_nsec, &data_type, &header_length) == 3) {
+	if (sscanf(buffer, RAMOOPS_KERNMSG_HDR TVSEC_FMT ".%lu-%c\n%n",
+		   &time->tv_sec, &time->tv_nsec, &data_type,
+		   &header_length) == 3) {
 		if (data_type == 'C')
 			*compressed = true;
 		else
 			*compressed = false;
-	} else if (sscanf(buffer, RAMOOPS_KERNMSG_HDR "%lu.%lu\n%n",
-			&time->tv_sec, &time->tv_nsec, &header_length) == 2) {
-			*compressed = false;
+	} else if (sscanf(buffer, RAMOOPS_KERNMSG_HDR TVSEC_FMT ".%lu\n%n",
+			  &time->tv_sec, &time->tv_nsec,
+			  &header_length) == 2) {
+		*compressed = false;
 	} else {
 		time->tv_sec = 0;
 		time->tv_nsec = 0;
@@ -360,7 +367,7 @@ static size_t ramoops_write_kmsg_hdr(struct persistent_ram_zone *prz,
 	char *hdr;
 	size_t len;
 
-	hdr = kasprintf(GFP_ATOMIC, RAMOOPS_KERNMSG_HDR "%lu.%lu-%c\n",
+	hdr = kasprintf(GFP_ATOMIC, RAMOOPS_KERNMSG_HDR TVSEC_FMT ".%lu-%c\n",
 		record->time.tv_sec,
 		record->time.tv_nsec / 1000,
 		record->compressed ? 'C' : 'D');
diff --git a/include/linux/pstore.h b/include/linux/pstore.h
index 61f806a7fe29..a15bc4d48752 100644
--- a/include/linux/pstore.h
+++ b/include/linux/pstore.h
@@ -71,7 +71,7 @@ struct pstore_record {
 	struct pstore_info	*psi;
 	enum pstore_type_id	type;
 	u64			id;
-	struct timespec		time;
+	struct timespec64	time;
 	char			*buf;
 	ssize_t			size;
 	ssize_t			ecc_notice_size;
-- 
2.17.0


-- 
Kees Cook
Pixel Security

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH v2] pstore: Convert internal records to timespec64
  2018-05-14 22:50 [PATCH v2] pstore: Convert internal records to timespec64 Kees Cook
@ 2018-05-15 17:42 ` Deepa Dinamani
  2018-05-30 15:06 ` Arnd Bergmann
  1 sibling, 0 replies; 3+ messages in thread
From: Deepa Dinamani @ 2018-05-15 17:42 UTC (permalink / raw)
  To: Kees Cook
  Cc: Linux Kernel Mailing List, Anton Vorontsov, Colin Cross, Tony Luck

Acked-by: Deepa Dinamani <deepa.kernel@gmail.com>

I think for 4.18, I can include it as part of my series.

Thanks,
-Deepa

On Mon, May 14, 2018 at 3:50 PM, Kees Cook <keescook@chromium.org> wrote:
> This prepares pstore for converting the VFS layer to timespec64.
>
> Signed-off-by: Kees Cook <keescook@chromium.org>
> ---
> I can carry this for v4.18 unless you'd like to do it as part of your series?
> ---
>  drivers/firmware/efi/efi-pstore.c | 27 ++++++++++++++-------------
>  fs/pstore/inode.c                 |  3 ++-
>  fs/pstore/platform.c              |  2 +-
>  fs/pstore/ram.c                   | 21 ++++++++++++++-------
>  include/linux/pstore.h            |  2 +-
>  5 files changed, 32 insertions(+), 23 deletions(-)
>
> diff --git a/drivers/firmware/efi/efi-pstore.c b/drivers/firmware/efi/efi-pstore.c
> index 5a0fa939d70f..cfe87b465819 100644
> --- a/drivers/firmware/efi/efi-pstore.c
> +++ b/drivers/firmware/efi/efi-pstore.c
> @@ -28,10 +28,9 @@ static int efi_pstore_close(struct pstore_info *psi)
>         return 0;
>  }
>
> -static inline u64 generic_id(unsigned long timestamp,
> -                            unsigned int part, int count)
> +static inline u64 generic_id(u64 timestamp, unsigned int part, int count)
>  {
> -       return ((u64) timestamp * 100 + part) * 1000 + count;
> +       return (timestamp * 100 + part) * 1000 + count;
>  }
>
>  static int efi_pstore_read_func(struct efivar_entry *entry,
> @@ -42,7 +41,8 @@ static int efi_pstore_read_func(struct efivar_entry *entry,
>         int i;
>         int cnt;
>         unsigned int part;
> -       unsigned long time, size;
> +       unsigned long size;
> +       u64 time;
>
>         if (efi_guidcmp(entry->var.VendorGuid, vendor))
>                 return 0;
> @@ -50,7 +50,7 @@ static int efi_pstore_read_func(struct efivar_entry *entry,
>         for (i = 0; i < DUMP_NAME_LEN; i++)
>                 name[i] = entry->var.VariableName[i];
>
> -       if (sscanf(name, "dump-type%u-%u-%d-%lu-%c",
> +       if (sscanf(name, "dump-type%u-%u-%d-%llu-%c",
>                    &record->type, &part, &cnt, &time, &data_type) == 5) {
>                 record->id = generic_id(time, part, cnt);
>                 record->part = part;
> @@ -62,7 +62,7 @@ static int efi_pstore_read_func(struct efivar_entry *entry,
>                 else
>                         record->compressed = false;
>                 record->ecc_notice_size = 0;
> -       } else if (sscanf(name, "dump-type%u-%u-%d-%lu",
> +       } else if (sscanf(name, "dump-type%u-%u-%d-%llu",
>                    &record->type, &part, &cnt, &time) == 4) {
>                 record->id = generic_id(time, part, cnt);
>                 record->part = part;
> @@ -71,7 +71,7 @@ static int efi_pstore_read_func(struct efivar_entry *entry,
>                 record->time.tv_nsec = 0;
>                 record->compressed = false;
>                 record->ecc_notice_size = 0;
> -       } else if (sscanf(name, "dump-type%u-%u-%lu",
> +       } else if (sscanf(name, "dump-type%u-%u-%llu",
>                           &record->type, &part, &time) == 3) {
>                 /*
>                  * Check if an old format,
> @@ -250,9 +250,10 @@ static int efi_pstore_write(struct pstore_record *record)
>         /* 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",
> +       snprintf(name, sizeof(name), "dump-type%u-%u-%d-%lld-%c",
>                  record->type, record->part, record->count,
> -                record->time.tv_sec, record->compressed ? 'C' : 'D');
> +                (long long)record->time.tv_sec,
> +                record->compressed ? 'C' : 'D');
>
>         for (i = 0; i < DUMP_NAME_LEN; i++)
>                 efi_name[i] = name[i];
> @@ -327,15 +328,15 @@ 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",
> +       snprintf(name, sizeof(name), "dump-type%u-%u-%d-%lld",
>                  record->type, record->part, record->count,
> -                record->time.tv_sec);
> +                (long long)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);
> +       snprintf(name, sizeof(name), "dump-type%u-%u-%lld",
> +               record->type, record->part, (long long)record->time.tv_sec);
>         ret = efi_pstore_erase_name(name);
>
>         return ret;
> diff --git a/fs/pstore/inode.c b/fs/pstore/inode.c
> index 5fcb845b9fec..75afe5eb0574 100644
> --- a/fs/pstore/inode.c
> +++ b/fs/pstore/inode.c
> @@ -392,7 +392,8 @@ int pstore_mkfile(struct dentry *root, struct pstore_record *record)
>         inode->i_private = private;
>
>         if (record->time.tv_sec)
> -               inode->i_mtime = inode->i_ctime = record->time;
> +               inode->i_mtime = inode->i_ctime =
> +                       timespec64_to_timespec(record->time);
>
>         d_add(dentry, inode);
>
> diff --git a/fs/pstore/platform.c b/fs/pstore/platform.c
> index dc720573fd53..c238ab8ba31d 100644
> --- a/fs/pstore/platform.c
> +++ b/fs/pstore/platform.c
> @@ -328,7 +328,7 @@ void pstore_record_init(struct pstore_record *record,
>         record->psi = psinfo;
>
>         /* Report zeroed timestamp if called before timekeeping has resumed. */
> -       record->time = ns_to_timespec(ktime_get_real_fast_ns());
> +       record->time = ns_to_timespec64(ktime_get_real_fast_ns());
>  }
>
>  /*
> diff --git a/fs/pstore/ram.c b/fs/pstore/ram.c
> index 49b2bc114868..69e893076ab7 100644
> --- a/fs/pstore/ram.c
> +++ b/fs/pstore/ram.c
> @@ -38,6 +38,11 @@
>
>  #define RAMOOPS_KERNMSG_HDR "===="
>  #define MIN_MEM_SIZE 4096UL
> +#if __BITS_PER_LONG == 64
> +# define TVSEC_FMT "%ld"
> +#else
> +# define TVSEC_FMT "%lld"
> +#endif
>
>  static ulong record_size = MIN_MEM_SIZE;
>  module_param(record_size, ulong, 0400);
> @@ -153,21 +158,23 @@ ramoops_get_next_prz(struct persistent_ram_zone *przs[], uint *c, uint max,
>         return prz;
>  }
>
> -static int ramoops_read_kmsg_hdr(char *buffer, struct timespec *time,
> +static int ramoops_read_kmsg_hdr(char *buffer, struct timespec64 *time,
>                                   bool *compressed)
>  {
>         char data_type;
>         int header_length = 0;
>
> -       if (sscanf(buffer, RAMOOPS_KERNMSG_HDR "%lu.%lu-%c\n%n", &time->tv_sec,
> -                       &time->tv_nsec, &data_type, &header_length) == 3) {
> +       if (sscanf(buffer, RAMOOPS_KERNMSG_HDR TVSEC_FMT ".%lu-%c\n%n",
> +                  &time->tv_sec, &time->tv_nsec, &data_type,
> +                  &header_length) == 3) {
>                 if (data_type == 'C')
>                         *compressed = true;
>                 else
>                         *compressed = false;
> -       } else if (sscanf(buffer, RAMOOPS_KERNMSG_HDR "%lu.%lu\n%n",
> -                       &time->tv_sec, &time->tv_nsec, &header_length) == 2) {
> -                       *compressed = false;
> +       } else if (sscanf(buffer, RAMOOPS_KERNMSG_HDR TVSEC_FMT ".%lu\n%n",
> +                         &time->tv_sec, &time->tv_nsec,
> +                         &header_length) == 2) {
> +               *compressed = false;
>         } else {
>                 time->tv_sec = 0;
>                 time->tv_nsec = 0;
> @@ -360,7 +367,7 @@ static size_t ramoops_write_kmsg_hdr(struct persistent_ram_zone *prz,
>         char *hdr;
>         size_t len;
>
> -       hdr = kasprintf(GFP_ATOMIC, RAMOOPS_KERNMSG_HDR "%lu.%lu-%c\n",
> +       hdr = kasprintf(GFP_ATOMIC, RAMOOPS_KERNMSG_HDR TVSEC_FMT ".%lu-%c\n",
>                 record->time.tv_sec,
>                 record->time.tv_nsec / 1000,
>                 record->compressed ? 'C' : 'D');
> diff --git a/include/linux/pstore.h b/include/linux/pstore.h
> index 61f806a7fe29..a15bc4d48752 100644
> --- a/include/linux/pstore.h
> +++ b/include/linux/pstore.h
> @@ -71,7 +71,7 @@ struct pstore_record {
>         struct pstore_info      *psi;
>         enum pstore_type_id     type;
>         u64                     id;
> -       struct timespec         time;
> +       struct timespec64       time;
>         char                    *buf;
>         ssize_t                 size;
>         ssize_t                 ecc_notice_size;
> --
> 2.17.0
>
>
> --
> Kees Cook
> Pixel Security

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH v2] pstore: Convert internal records to timespec64
  2018-05-14 22:50 [PATCH v2] pstore: Convert internal records to timespec64 Kees Cook
  2018-05-15 17:42 ` Deepa Dinamani
@ 2018-05-30 15:06 ` Arnd Bergmann
  1 sibling, 0 replies; 3+ messages in thread
From: Arnd Bergmann @ 2018-05-30 15:06 UTC (permalink / raw)
  To: Kees Cook
  Cc: Deepa Dinamani, Linux Kernel Mailing List, Anton Vorontsov,
	Colin Cross, Tony Luck, Thomas Gleixner

On Tue, May 15, 2018 at 12:50 AM, Kees Cook <keescook@chromium.org> wrote:
> diff --git a/fs/pstore/ram.c b/fs/pstore/ram.c
> index 49b2bc114868..69e893076ab7 100644
> --- a/fs/pstore/ram.c
> +++ b/fs/pstore/ram.c
> @@ -38,6 +38,11 @@
>
>  #define RAMOOPS_KERNMSG_HDR "===="
>  #define MIN_MEM_SIZE 4096UL
> +#if __BITS_PER_LONG == 64
> +# define TVSEC_FMT "%ld"
> +#else
> +# define TVSEC_FMT "%lld"
> +#endif
>
>  static ulong record_size = MIN_MEM_SIZE;
>  module_param(record_size, ulong, 0400);

Unfortunately, this bit conflicts with my patch "timekeeping: Remove
timespec64 hack"
that Thomas queued in tip: https://lkml.org/lkml/2018/5/19/115, causing
a harmless but annoying build warning on 64-bit machines:

fs/pstore/ram.c: In function 'ramoops_read_kmsg_hdr':
fs/pstore/ram.c:39:29: error: format '%ld' expects argument of type
'long int *', but argument 3 has type 'time64_t *' {aka 'long long int
*'} [-Werror=format=]
 #define RAMOOPS_KERNMSG_HDR "===="
                             ^~~~~~
fs/pstore/ram.c:167:21: note: in expansion of macro 'RAMOOPS_KERNMSG_HDR'

The tip/timers/y2038 branch is currently not part of linux-next. It should be
easy to fix it up with a patch on top that open-codes TVSEC_FMT as %lld,
and adds a cast to 'long long' like every other such printk has at the moment
(allowing them the casts to be mass-removed after my patch is merged).

      Arnd

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2018-05-30 15:06 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-05-14 22:50 [PATCH v2] pstore: Convert internal records to timespec64 Kees Cook
2018-05-15 17:42 ` Deepa Dinamani
2018-05-30 15:06 ` Arnd Bergmann

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).