Re: [PATCH v2] kdb: unify CMD_BUFLEN definition into kdb_private.h

[David Laight]

On Wed, 17 Jun 2026 14:16:41 -0700
Doug Anderson <dianders@xxxxxxxxxxxx> wrote:

> Hi,
> 
> On Tue, Jun 16, 2026 at 7:28 PM Naveen Kumar Chaudhary
> <naveen.osdev@xxxxxxxxx> wrote:
> >
> > CMD_BUFLEN was defined separately in kdb_io.c (256) and kdb_main.c
> > (200), causing kdb_main.c to use the wrong size when formatting the
> > prompt string into kdb_prompt_str (which is 256 bytes).
> >
> > Move CMD_BUFLEN (256) into kdb_private.h so all users share a single
> > consistent definition, and remove the local definitions from both
> > files.
> >
> > Fixes: 5d5314d6795f ("kdb: core for kgdb back end (1 of 2)")
> > Signed-off-by: Naveen Kumar Chaudhary <naveen.osdev@xxxxxxxxx>
> > ---
> >  kernel/debug/kdb/kdb_io.c      | 1 -
> >  kernel/debug/kdb/kdb_main.c    | 6 ++----
> >  kernel/debug/kdb/kdb_private.h | 3 ++-
> >  3 files changed, 4 insertions(+), 6 deletions(-)
> >
> > diff --git a/kernel/debug/kdb/kdb_io.c b/kernel/debug/kdb/kdb_io.c
> > index c399f11740ef..f5b1b7d4c9c8 100644
> > --- a/kernel/debug/kdb/kdb_io.c
> > +++ b/kernel/debug/kdb/kdb_io.c
> > @@ -22,7 +22,6 @@
> >  #include <linux/kallsyms.h>
> >  #include "kdb_private.h"
> >
> > -#define CMD_BUFLEN 256
> >  char kdb_prompt_str[CMD_BUFLEN];
> >
> >  int kdb_trap_printk;
> > diff --git a/kernel/debug/kdb/kdb_main.c b/kernel/debug/kdb/kdb_main.c
> > index ddce56b47b25..ca0126db9850 100644
> > --- a/kernel/debug/kdb/kdb_main.c
> > +++ b/kernel/debug/kdb/kdb_main.c
> > @@ -783,8 +783,6 @@ static int kdb_exec_defcmd(int argc, const char **argv)
> >
> >  /* Command history */
> >  #define KDB_CMD_HISTORY_COUNT  32
> > -#define CMD_BUFLEN             200     /* kdb_printf: max printline
> > -                                        * size == 256 */  
> 
> Maybe Daniel will know more; otherwise, I need to spend more time
> digging. ...but the comment above (that you're deleting) makes me
> believe that 200 was purposely chosen to be a number that was under
> 256. It sounds as if maybe they're keeping some buffers at 200 so that
> there'e enough extra space to print the buffer plus some extra stuff?
> 
> Maybe safer to keep the number at 200?

I bet the prompt+command has to fit in 256 bytes?
Who wants a prompt that is more than (about) 20 characters anyway?
Why do some systems default to shell prompts that include the full
directory name?

FWIW there are a few other very strange strcpy() in kdbg that I'm pretty
sure can actually overwrite the end of the buffer.
(Ones that put a special marker string (IIRC "kdbg") into a buffer.)

	David

> 
> 
> >  static unsigned int cmd_head, cmd_tail;
> >  static unsigned int cmdptr;
> >  static char cmd_hist[KDB_CMD_HISTORY_COUNT][CMD_BUFLEN];
> > @@ -1265,8 +1263,8 @@ static int kdb_local(kdb_reason_t reason, int error, struct pt_regs *regs,
> >
> >  do_full_getstr:
> >                 /* PROMPT can only be set if we have MEM_READ permission. */
> > -               snprintf(kdb_prompt_str, CMD_BUFLEN, kdbgetenv("PROMPT"),
> > -                        raw_smp_processor_id());
> > +               snprintf(kdb_prompt_str, CMD_BUFLEN,
> > +                        kdbgetenv("PROMPT"), raw_smp_processor_id());  
> 
> Unrelated whitespace change. Drop from your patch.
> 
> 
> >                 /*
> >                  * Fetch command from keyboard
> > diff --git a/kernel/debug/kdb/kdb_private.h b/kernel/debug/kdb/kdb_private.h
> > index 92a28b8ab604..722e8aa50724 100644
> > --- a/kernel/debug/kdb/kdb_private.h
> > +++ b/kernel/debug/kdb/kdb_private.h
> > @@ -225,7 +225,8 @@ extern void kdb_kbd_cleanup_state(void);
> >  #define kdb_kbd_cleanup_state()
> >  #endif /* ! CONFIG_KDB_KEYBOARD */
> >
> > -extern char kdb_prompt_str[];
> > +#define CMD_BUFLEN 256
> > +extern char kdb_prompt_str[CMD_BUFLEN];  
> 
> Now that this is in a header file, a slightly less generic name would
> be good. Maybe rename to KDB_BUFLEN"
>