[bug report] bootconfig: init: Allow admin to use bootconfig for kernel command line

Dan Carpenter <[email protected]>
Newsgroups gmane.linux.kernel.janitors
Message-ID <[email protected]>
Hello Masami Hiramatsu,

Commit 51887d03aca1 ("bootconfig: init: Allow admin to use bootconfig
for kernel command line") from Jan 11, 2020 (linux-next), leads to
the following Smatch static checker warning:

	init/main.c:368 xbc_snprint_cmdline()
	use scnprintf() instead of snprintf()

init/main.c
    331 static int __init xbc_snprint_cmdline(char *buf, size_t size,
    332                                       struct xbc_node *root)
    333 {
    334         struct xbc_node *knode, *vnode;
    335         char *end = buf + size;
    336         const char *val, *q;
    337         int ret;
    338 
    339         xbc_node_for_each_key_value(root, knode, val) {
    340                 ret = xbc_node_compose_key_after(root, knode,
    341                                         xbc_namebuf, XBC_KEYLEN_MAX);
    342                 if (ret < 0)
    343                         return ret;
    344 
    345                 vnode = xbc_node_get_child(knode);
    346                 if (!vnode) {
    347                         ret = snprintf(buf, rest(buf, end), "%s ", xbc_namebuf);
    348                         if (ret < 0)
    349                                 return ret;
    350                         buf += ret;

In user space snprintf() can return negative, but in the kernel, no.
It returns the number of bytes (not counting the NUL terminator) which
would have been copied if there were enough space.  So maybe you want
to do something like:

	remain = rest(buf, end);
	ret = snprintf(buf, rest(buf, end), "%s ", xbc_namebuf);
	if (ret >= remain)
		return -ENOSPC;

Or maybe you might want to use scnprintf() which returns the number of
bytes actually copied.  Otherwise bug ends up pointing to beyond the end
of the buffer.

    351                         continue;
    352                 }
    353                 xbc_array_for_each_value(vnode, val) {
    354                         /*
    355                          * For prettier and more readable /proc/cmdline, only
    356                          * quote the value when necessary, i.e. when it contains
    357                          * whitespace.
    358                          */
    359                         q = strpbrk(val, " \t\r\n") ? "\"" : "";
    360                         ret = snprintf(buf, rest(buf, end), "%s=%s%s%s ",
                                ^^^^^^^^^^^^^^^
Same.

    361                                        xbc_namebuf, q, val, q);
    362                         if (ret < 0)
    363                                 return ret;
    364                         buf += ret;
    365                 }
    366         }
    367 
--> 368         return buf - (end - size);
    369 }

This email is a free service from the Smatch-CI project [smatch.sf.net].

regards,
dan carpenter
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.