2022-05-09 03:03:38

by James Bottomley

[permalink] [raw]
Subject: Re: [PATCH v1 1/3] kallsyms: avoid hardcoding the buffer size

On Thu, 2022-05-05 at 21:16 +0200, Miguel Ojeda wrote:
> From: Boqun Feng <[email protected]>
>
> This makes it easier to update the size later on.
>
> Furthermore, a static assert is added to ensure both are updated
> when that happens. The relationship used is one that keeps the new
> size (512+1) close to the original buffer size (500).
>
> Signed-off-by: Boqun Feng <[email protected]>
> Co-developed-by: Miguel Ojeda <[email protected]>
> Signed-off-by: Miguel Ojeda <[email protected]>
> ---
> scripts/kallsyms.c | 16 +++++++++++++---
> 1 file changed, 13 insertions(+), 3 deletions(-)
>
> diff --git a/scripts/kallsyms.c b/scripts/kallsyms.c
> index 8caabddf817c..880c4404731b 100644
> --- a/scripts/kallsyms.c
> +++ b/scripts/kallsyms.c
> @@ -27,8 +27,18 @@
>
> #define ARRAY_SIZE(arr) (sizeof(arr) / sizeof(arr[0]))
>
> +#define _stringify_1(x) #x
> +#define _stringify(x) _stringify_1(x)
> +
> #define KSYM_NAME_LEN 128
>
> +/* A substantially bigger size than the current maximum. */
> +#define KSYM_NAME_LEN_BUFFER 512
> +_Static_assert(
> + KSYM_NAME_LEN_BUFFER == KSYM_NAME_LEN * 4,
> + "Please keep KSYM_NAME_LEN_BUFFER in sync with KSYM_NAME_LEN"
> +);
> +
> struct sym_entry {
> unsigned long long addr;
> unsigned int len;
> @@ -197,15 +207,15 @@ static void check_symbol_range(const char *sym,
> unsigned long long addr,
>
> static struct sym_entry *read_symbol(FILE *in)
> {
> - char name[500], type;
> + char name[KSYM_NAME_LEN_BUFFER+1], type;

When you raise KSYM_NAME_LEN to 512, this on stack allocation becomes
2049 bytes. How did you manage not to trigger the frame size warning,
which is 1024 on 32 bit and 2048 on 64 bit by default?

James