On Mon 2023-05-29 16:50:45, Miguel Ojeda wrote:

On Mon, May 29, 2023 at 1:08 PM Maninder Singh [off-list ref] wrote:

quoted

I Will add co-developed-by` tag.
because this change was identified while we were working on kallsyms some time back.
https://lore.kernel.org/lkml/YonTOL4zC4CytVrn@infradead.org/t/ (local)

this patch set is pending and we will start working on that again, so i thought better
to send bugfix first.

Sounds good to me!

(Fixed Wedson's email address)

quoted

Yes, I think second buffer was not related to kallsyms, so I have not touched that.

Kees: what is the current stance on `[static N]` parameters? Something like:

    const char *kallsyms_lookup(unsigned long addr,
                                unsigned long *symbolsize,
                                unsigned long *offset,
    -                           char **modname, char *namebuf);
    +                           char **modname, char namebuf[static
KSYM_NAME_LEN]);

makes the compiler complain about cases like these (even if trivial):

    arch/powerpc/xmon/xmon.c:1711:10: error: array argument is too small;
        contains 128 elements, callee requires at least 512
[-Werror,-Warray-bounds]
            name = kallsyms_lookup(pc, &size, &offset, NULL, tmpstr);
                 ^                                           ~~~~~~
    ./include/linux/kallsyms.h:86:29: note: callee declares array
parameter as static here
            char **modname, char namebuf[static KSYM_NAME_LEN]);
                                 ^      ~~~~~~~~~~~~~~~~~~~~~~

But I only see 2 files in the kernel using `[static N]` (from 2020 and
2021). Should something else be used instead (e.g. `__counted_by`),
even if constexpr-sized?.

Also, I went through the other callers to `kallsyms_lookup` to see
other issues -- one I am not sure about is `fetch_store_symstring` in
`kernel/trace/trace_probe_tmpl.h`. Steven/Masami: is that "with max
length" in the function docs enough? Is it 0xffff?

The best solution would be to pass the buffer size as an extra
parameter. Especially when some code passes buffers that are
allocated/reserved dynamically.

Sigh, I am not sure how many changes it would require in kallsyms
API and all the callers. But it would be really appreciated, IMHO.

Best Regards,
Petr