Re: [PATCH 4.19 051/168] dyndbg: fix a BUG_ON in ddebug_describe_flags

Pavel Machek
2020-08-18T09:51:31Z

[-- Attachment #1: Type: text/plain, Size: 3233 bytes --]

On Mon 2020-08-17 17:16:22, Greg Kroah-Hartman wrote:
> From: Jim Cromie <jim.cromie@gmail.com>
>
> [ Upstream commit f678ce8cc3cb2ad29df75d8824c74f36398ba871 ]
>
> ddebug_describe_flags() currently fills a caller provided string buffer,
> after testing its size (also passed) in a BUG_ON. Fix this by
> replacing them with a known-big-enough string buffer wrapped in a
> struct, and passing that instead.
>
> Also simplify ddebug_describe_flags() flags parameter from a struct to
> a member in that struct, and hoist the member deref up to the caller.
> This makes the function reusable (soon) where flags are unpacked.

Original code was correct, passing explicit size, this passes strange
structure. BUG_ON can never trigger in the origianl code, so this is
not a bugfix.

Best regards,
Pavel

> +++ b/lib/dynamic_debug.c
> @@ -85,22 +85,22 @@ static struct { unsigned flag:8; char opt_char; } opt_array[] = {
> { _DPRINTK_FLAGS_NONE, '_' },
> };
>
> +struct flagsbuf { char buf[ARRAY_SIZE(opt_array)+1]; };
> +
> /* format a string into buf[] which describes the _ddebug's flags */
> -static char *ddebug_describe_flags(struct _ddebug *dp, char *buf,
> - size_t maxlen)
> +static char *ddebug_describe_flags(unsigned int flags, struct flagsbuf *fb)
> {
> - char *p = buf;
> + char *p = fb->buf;
> int i;
>
> - BUG_ON(maxlen < 6);
> for (i = 0; i < ARRAY_SIZE(opt_array); ++i)
> - if (dp->flags & opt_array[i].flag)
> + if (flags & opt_array[i].flag)
> *p++ = opt_array[i].opt_char;
> - if (p == buf)
> + if (p == fb->buf)
> *p++ = '_';
> *p = '\0';
>
> - return buf;
> + return fb->buf;
> }
>
> #define vpr_info(fmt, ...) \
> @@ -142,7 +142,7 @@ static int ddebug_change(const struct ddebug_query *query,
> struct ddebug_table *dt;
> unsigned int newflags;
> unsigned int nfound = 0;
> - char flagbuf[10];
> + struct flagsbuf fbuf;
>
> /* search for matching ddebugs */
> mutex_lock(&ddebug_lock);
> @@ -199,8 +199,7 @@ static int ddebug_change(const struct ddebug_query *query,
> vpr_info("changed %s:%d [%s]%s =%s\n",
> trim_prefix(dp->filename), dp->lineno,
> dt->mod_name, dp->function,
> - ddebug_describe_flags(dp, flagbuf,
> - sizeof(flagbuf)));
> + ddebug_describe_flags(dp->flags, &fbuf));
> }
> }
> mutex_unlock(&ddebug_lock);
> @@ -779,7 +778,7 @@ static int ddebug_proc_show(struct seq_file *m, void *p)
> {
> struct ddebug_iter *iter = m->private;
> struct _ddebug *dp = p;
> - char flagsbuf[10];
> + struct flagsbuf flags;
>
> vpr_info("called m=%p p=%p\n", m, p);
>
> @@ -792,7 +791,7 @@ static int ddebug_proc_show(struct seq_file *m, void *p)
> seq_printf(m, "%s:%u [%s]%s =%s \"",
> trim_prefix(dp->filename), dp->lineno,
> iter->table->mod_name, dp->function,
> - ddebug_describe_flags(dp, flagsbuf, sizeof(flagsbuf)));
> + ddebug_describe_flags(dp->flags, &flags));
> seq_escape(m, dp->format, "\t\r\n\"");
> seq_puts(m, "\"\n");
>

--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]