| Message ID | 20180530203024.2792802-1-arnd@arndb.de |
|---|---|
| State | Accepted |
| Commit | e264abeaf9daa3cde9aed8013a6f82b0370425e5 |
| Headers | show |
| Series | pstore: Remove bogus format string definition | expand |
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
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);
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