pstore: Remove bogus format string definition

Message ID 20180530203024.2792802-1-arnd@arndb.de
State Accepted
Commit e264abeaf9daa3cde9aed8013a6f82b0370425e5
Headers show
Series
  • pstore: Remove bogus format string definition
Related show

Commit Message

Arnd Bergmann May 30, 2018, 8:30 p.m.
The pstore conversion to timespec64 introduces its own method of passing
seconds into sscanf() and sprintf() type functions to work around the
timespec64 definition on 64-bit systems that redefine it to 'timespec'.

That hack is now finally getting removed, but that means we get a (harmless)
warning once both patches are merged:

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'

This removes the pstore specific workaround and uses the same method that
we have in place for all other functions that print a timespec64.

Related to this, I found that the kasprintf() output contains an incorrect
nanosecond values for any number starting with zeroes, and I adapt the
format string accordingly.

Link: https://lkml.org/lkml/2018/5/19/115
Link: https://lkml.org/lkml/2018/5/16/1080
Fixes: 0f0d83b99ef7 ("pstore: Convert internal records to timespec64")
Signed-off-by: Arnd Bergmann <arnd@arndb.de>

---
 fs/pstore/ram.c | 17 ++++++-----------
 1 file changed, 6 insertions(+), 11 deletions(-)

-- 
2.9.0

Comments

Kees Cook May 30, 2018, 9:37 p.m. | #1
On Wed, May 30, 2018 at 1:30 PM, Arnd Bergmann <arnd@arndb.de> wrote:
> The pstore conversion to timespec64 introduces its own method of passing

> seconds into sscanf() and sprintf() type functions to work around the

> timespec64 definition on 64-bit systems that redefine it to 'timespec'.

>

> That hack is now finally getting removed, but that means we get a (harmless)

> warning once both patches are merged:

>

> 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'

>

> This removes the pstore specific workaround and uses the same method that

> we have in place for all other functions that print a timespec64.

>

> Related to this, I found that the kasprintf() output contains an incorrect

> nanosecond values for any number starting with zeroes, and I adapt the

> format string accordingly.

>

> Link: https://lkml.org/lkml/2018/5/19/115

> Link: https://lkml.org/lkml/2018/5/16/1080

> Fixes: 0f0d83b99ef7 ("pstore: Convert internal records to timespec64")

> Signed-off-by: Arnd Bergmann <arnd@arndb.de>


Acked-by: Kees Cook <keescook@chromium.org>


Please feel free to carry this patch in which ever series/tree it is needed. :P

-Kees

> ---

>  fs/pstore/ram.c | 17 ++++++-----------

>  1 file changed, 6 insertions(+), 11 deletions(-)

>

> diff --git a/fs/pstore/ram.c b/fs/pstore/ram.c

> index 69e893076ab7..bbd1e357c23d 100644

> --- a/fs/pstore/ram.c

> +++ b/fs/pstore/ram.c

> @@ -38,11 +38,6 @@

>

>  #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);

> @@ -164,15 +159,15 @@ static int ramoops_read_kmsg_hdr(char *buffer, struct timespec64 *time,

>         char data_type;

>         int header_length = 0;

>

> -       if (sscanf(buffer, RAMOOPS_KERNMSG_HDR TVSEC_FMT ".%lu-%c\n%n",

> -                  &time->tv_sec, &time->tv_nsec, &data_type,

> +       if (sscanf(buffer, RAMOOPS_KERNMSG_HDR "%lld.%lu-%c\n%n",

> +                  (time64_t *)&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 TVSEC_FMT ".%lu\n%n",

> -                         &time->tv_sec, &time->tv_nsec,

> +       } else if (sscanf(buffer, RAMOOPS_KERNMSG_HDR "%lld.%lu\n%n",

> +                         (time64_t *)&time->tv_sec, &time->tv_nsec,

>                           &header_length) == 2) {

>                 *compressed = false;

>         } else {

> @@ -367,8 +362,8 @@ static size_t ramoops_write_kmsg_hdr(struct persistent_ram_zone *prz,

>         char *hdr;

>         size_t len;

>

> -       hdr = kasprintf(GFP_ATOMIC, RAMOOPS_KERNMSG_HDR TVSEC_FMT ".%lu-%c\n",

> -               record->time.tv_sec,

> +       hdr = kasprintf(GFP_ATOMIC, RAMOOPS_KERNMSG_HDR "%lld.%06lu-%c\n",

> +               (time64_t)record->time.tv_sec,

>                 record->time.tv_nsec / 1000,

>                 record->compressed ? 'C' : 'D');

>         WARN_ON_ONCE(!hdr);

> --

> 2.9.0

>




-- 
Kees Cook
Pixel Security

Patch

diff --git a/fs/pstore/ram.c b/fs/pstore/ram.c
index 69e893076ab7..bbd1e357c23d 100644
--- a/fs/pstore/ram.c
+++ b/fs/pstore/ram.c
@@ -38,11 +38,6 @@ 
 
 #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);
@@ -164,15 +159,15 @@  static int ramoops_read_kmsg_hdr(char *buffer, struct timespec64 *time,
 	char data_type;
 	int header_length = 0;
 
-	if (sscanf(buffer, RAMOOPS_KERNMSG_HDR TVSEC_FMT ".%lu-%c\n%n",
-		   &time->tv_sec, &time->tv_nsec, &data_type,
+	if (sscanf(buffer, RAMOOPS_KERNMSG_HDR "%lld.%lu-%c\n%n",
+		   (time64_t *)&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 TVSEC_FMT ".%lu\n%n",
-			  &time->tv_sec, &time->tv_nsec,
+	} else if (sscanf(buffer, RAMOOPS_KERNMSG_HDR "%lld.%lu\n%n",
+			  (time64_t *)&time->tv_sec, &time->tv_nsec,
 			  &header_length) == 2) {
 		*compressed = false;
 	} else {
@@ -367,8 +362,8 @@  static size_t ramoops_write_kmsg_hdr(struct persistent_ram_zone *prz,
 	char *hdr;
 	size_t len;
 
-	hdr = kasprintf(GFP_ATOMIC, RAMOOPS_KERNMSG_HDR TVSEC_FMT ".%lu-%c\n",
-		record->time.tv_sec,
+	hdr = kasprintf(GFP_ATOMIC, RAMOOPS_KERNMSG_HDR "%lld.%06lu-%c\n",
+		(time64_t)record->time.tv_sec,
 		record->time.tv_nsec / 1000,
 		record->compressed ? 'C' : 'D');
 	WARN_ON_ONCE(!hdr);