[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <ZHWub0ibU7etLnXK@alley>
Date: Tue, 30 May 2023 10:06:07 +0200
From: Petr Mladek <pmladek@...e.com>
To: Miguel Ojeda <miguel.ojeda.sandonis@...il.com>
Cc: maninder1.s@...sung.com,
"keescook@...omium.org" <keescook@...omium.org>,
Steven Rostedt <rostedt@...dmis.org>,
Masami Hiramatsu <mhiramat@...nel.org>,
"bcain@...cinc.com" <bcain@...cinc.com>,
"mpe@...erman.id.au" <mpe@...erman.id.au>,
"npiggin@...il.com" <npiggin@...il.com>,
"christophe.leroy@...roup.eu" <christophe.leroy@...roup.eu>,
"nathanl@...ux.ibm.com" <nathanl@...ux.ibm.com>,
"ustavoars@...nel.org" <ustavoars@...nel.org>,
"alex.gaynor@...il.com" <alex.gaynor@...il.com>,
"gary@...yguo.net" <gary@...yguo.net>,
"ojeda@...nel.org" <ojeda@...nel.org>,
"linux-hexagon@...r.kernel.org" <linux-hexagon@...r.kernel.org>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
"linuxppc-dev@...ts.ozlabs.org" <linuxppc-dev@...ts.ozlabs.org>,
Onkarnath <onkarnath.1@...sung.com>,
Wedson Almeida Filho <wedsonaf@...il.com>
Subject: Re: [PATCH 1/1] arch:hexagon/powerpc: use KSYM_NAME_LEN in array size
On Mon 2023-05-29 16:50:45, Miguel Ojeda wrote:
> On Mon, May 29, 2023 at 1:08 PM Maninder Singh <maninder1.s@...sung.com> wrote:
> >
> > 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/
> >
> > 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)
>
> > 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
Powered by blists - more mailing lists