| Message ID | 20180613092919.24075-1-thierry.escande@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | [v2] lib/test_printf.c: call wait_for_random_bytes() before plain %p tests | expand |
On Wed, Jun 13, 2018 at 12:29 PM, Thierry Escande <thierry.escande@linaro.org> wrote: > If the test_printf module is loaded before the crng is initialized, the > plain 'p' tests will fail because the printed address will not be hashed > and the buffer will contain "(ptrval)" instead. > Since we cannot wait for the crng to be initialized for an undefined > time, both plain 'p' tests now accept the string "(ptrval)" as a valid > result and print a warning message. There are two possibilities: 1. (ptrval) for 32-bit case 2. (____ptrval____) for 64-bit case. -- With Best Regards, Andy Shevchenko
On 13/06/2018 13:22, Andy Shevchenko wrote: > On Wed, Jun 13, 2018 at 12:29 PM, Thierry Escande > <thierry.escande@linaro.org> wrote: >> If the test_printf module is loaded before the crng is initialized, the >> plain 'p' tests will fail because the printed address will not be hashed >> and the buffer will contain "(ptrval)" instead. >> Since we cannot wait for the crng to be initialized for an undefined >> time, both plain 'p' tests now accept the string "(ptrval)" as a valid >> result and print a warning message. > > There are two possibilities: > 1. (ptrval) for 32-bit case > 2. (____ptrval____) for 64-bit case. From lib/vsprintf.c, ptr_to_id() puts "(ptrval)" into the buffer, then it gets left-padded with spaces by widen_string(). Regards, Thierry
On Wed, Jun 13, 2018 at 5:59 PM, Thierry Escande <thierry.escande@linaro.org> wrote: > On 13/06/2018 13:22, Andy Shevchenko wrote: >> >> On Wed, Jun 13, 2018 at 12:29 PM, Thierry Escande >> <thierry.escande@linaro.org> wrote: >>> >>> If the test_printf module is loaded before the crng is initialized, the >>> plain 'p' tests will fail because the printed address will not be hashed >>> and the buffer will contain "(ptrval)" instead. >>> Since we cannot wait for the crng to be initialized for an undefined >>> time, both plain 'p' tests now accept the string "(ptrval)" as a valid >>> result and print a warning message. >> >> >> There are two possibilities: >> 1. (ptrval) for 32-bit case >> 2. (____ptrval____) for 64-bit case. > > > From lib/vsprintf.c, ptr_to_id() puts "(ptrval)" into the buffer, then it > gets left-padded with spaces by widen_string(). Which kernel version you are trying to fix? What I see for a long time in linux-next: static char *ptr_to_id(char *buf, char *end, void *ptr, struct printf_spec spec) { const char *str = sizeof(ptr) == 8 ? "(____ptrval____)" : "(ptrval)"; ... brought by the commit 91efafb1dd8f ("lib/vsprintf: Replace space with '_' before crng is ready"). -- With Best Regards, Andy Shevchenko
On 13/06/2018 18:30, Andy Shevchenko wrote: > On Wed, Jun 13, 2018 at 5:59 PM, Thierry Escande > <thierry.escande@linaro.org> wrote: >> On 13/06/2018 13:22, Andy Shevchenko wrote: >>> >>> On Wed, Jun 13, 2018 at 12:29 PM, Thierry Escande >>> <thierry.escande@linaro.org> wrote: >>>> >>>> If the test_printf module is loaded before the crng is initialized, the >>>> plain 'p' tests will fail because the printed address will not be hashed >>>> and the buffer will contain "(ptrval)" instead. >>>> Since we cannot wait for the crng to be initialized for an undefined >>>> time, both plain 'p' tests now accept the string "(ptrval)" as a valid >>>> result and print a warning message. >>> >>> >>> There are two possibilities: >>> 1. (ptrval) for 32-bit case >>> 2. (____ptrval____) for 64-bit case. >> >> >> From lib/vsprintf.c, ptr_to_id() puts "(ptrval)" into the buffer, then it >> gets left-padded with spaces by widen_string(). > > Which kernel version you are trying to fix? > > What I see for a long time in linux-next: > > static char *ptr_to_id(char *buf, char *end, void *ptr, struct printf_spec spec) > { > const char *str = sizeof(ptr) == 8 ? "(____ptrval____)" : "(ptrval)"; > ... > > brought by the commit 91efafb1dd8f ("lib/vsprintf: Replace space with > '_' before crng is ready"). I missed that one as it's not in v4.17. My bad. Regards, Thierry
diff --git a/lib/test_printf.c b/lib/test_printf.c index 71ebfa43ad05..a625e3749566 100644 --- a/lib/test_printf.c +++ b/lib/test_printf.c @@ -206,6 +206,7 @@ test_string(void) #define PTR_WIDTH 16 #define PTR ((void *)0xffff0123456789ab) #define PTR_STR "ffff0123456789ab" +#define PTR_VAL_NO_CRNG " (ptrval)" #define ZEROS "00000000" /* hex 32 zero bits */ static int __init @@ -216,7 +217,16 @@ plain_format(void) nchars = snprintf(buf, PLAIN_BUF_SIZE, "%p", PTR); - if (nchars != PTR_WIDTH || strncmp(buf, ZEROS, strlen(ZEROS)) != 0) + if (nchars != PTR_WIDTH) + return -1; + + if (strncmp(buf, PTR_VAL_NO_CRNG, PTR_WIDTH) == 0) { + pr_warn("crng possibly not yet initialized. plain 'p' buffer contains \"%s\"", + PTR_VAL_NO_CRNG); + return 0; + } + + if (strncmp(buf, ZEROS, strlen(ZEROS)) != 0) return -1; return 0; @@ -227,6 +237,7 @@ plain_format(void) #define PTR_WIDTH 8 #define PTR ((void *)0x456789ab) #define PTR_STR "456789ab" +#define PTR_VAL_NO_CRNG "(ptrval)" static int __init plain_format(void) @@ -245,7 +256,16 @@ plain_hash(void) nchars = snprintf(buf, PLAIN_BUF_SIZE, "%p", PTR); - if (nchars != PTR_WIDTH || strncmp(buf, PTR_STR, PTR_WIDTH) == 0) + if (nchars != PTR_WIDTH) + return -1; + + if (strncmp(buf, PTR_VAL_NO_CRNG, PTR_WIDTH) == 0) { + pr_warn("crng possibly not yet initialized. plain 'p' buffer contains \"%s\"", + PTR_VAL_NO_CRNG); + return 0; + } + + if (strncmp(buf, PTR_STR, PTR_WIDTH) == 0) return -1; return 0;
If the test_printf module is loaded before the crng is initialized, the plain 'p' tests will fail because the printed address will not be hashed and the buffer will contain "(ptrval)" instead. Since we cannot wait for the crng to be initialized for an undefined time, both plain 'p' tests now accept the string "(ptrval)" as a valid result and print a warning message. Signed-off-by: Thierry Escande <thierry.escande@linaro.org> --- Change in v2: - Remove wait_for_random_bytes() usage - Removed Acked-by from Tobin as the proposed solution is not the same anymore. lib/test_printf.c | 24 ++++++++++++++++++++++-- 1 file changed, 22 insertions(+), 2 deletions(-) -- 2.14.1