* [PATCH 1/8] bootconfig: Set CONFIG_BOOT_CONFIG=n by default
2020-02-20 7:26 [PATCH 0/8] bootconfig: Update for the recent reports Masami Hiramatsu
@ 2020-02-20 7:26 ` Masami Hiramatsu
2020-02-20 8:16 ` Geert Uytterhoeven
2020-02-20 7:26 ` [PATCH 2/8] bootconfig: Add bootconfig magic word for indicating bootconfig explicitly Masami Hiramatsu
` (6 subsequent siblings)
7 siblings, 1 reply; 11+ messages in thread
From: Masami Hiramatsu @ 2020-02-20 7:26 UTC (permalink / raw)
To: Steven Rostedt
Cc: Geert Uytterhoeven, Borislav Petkov, LKML, Ingo Molnar,
Andrew Morton, Masami Hiramatsu, Peter Zijlstra
Set CONFIG_BOOT_CONFIG=n by default. This also warns
user if CONFIG_BOOT_CONFIG=n but "bootconfig" is given
in the kernel command line.
Suggested-by: Steven Rostedt <rostedt@goodmis.org>
Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org>
---
init/Kconfig | 2 +-
init/main.c | 8 ++++++++
kernel/trace/Kconfig | 3 ++-
3 files changed, 11 insertions(+), 2 deletions(-)
diff --git a/init/Kconfig b/init/Kconfig
index 4a672c6629d0..824f3763260d 100644
--- a/init/Kconfig
+++ b/init/Kconfig
@@ -1218,7 +1218,7 @@ endif
config BOOT_CONFIG
bool "Boot config support"
depends on BLK_DEV_INITRD
- default y
+ default n
help
Extra boot config allows system admin to pass a config file as
complemental extension of kernel cmdline when booting.
diff --git a/init/main.c b/init/main.c
index 59248717c925..680ff7123705 100644
--- a/init/main.c
+++ b/init/main.c
@@ -418,6 +418,14 @@ static void __init setup_boot_config(const char *cmdline)
}
#else
#define setup_boot_config(cmdline) do { } while (0)
+
+static int __init warn_bootconfig(char *str)
+{
+ pr_err("WARNING: 'bootconfig' found on the kernel command line but CONFIG_BOOTCONFIG is not set.\n");
+ return 0;
+}
+early_param("bootconfig", warn_bootconfig);
+
#endif
/* Change NUL term back to "=", to make "param" the whole string. */
diff --git a/kernel/trace/Kconfig b/kernel/trace/Kconfig
index 91e885194dbc..795c3e02d3f1 100644
--- a/kernel/trace/Kconfig
+++ b/kernel/trace/Kconfig
@@ -143,7 +143,8 @@ if FTRACE
config BOOTTIME_TRACING
bool "Boot-time Tracing support"
- depends on BOOT_CONFIG && TRACING
+ depends on TRACING
+ select BOOT_CONFIG
default y
help
Enable developer to setup ftrace subsystem via supplemental
^ permalink raw reply related [flat|nested] 11+ messages in thread* Re: [PATCH 1/8] bootconfig: Set CONFIG_BOOT_CONFIG=n by default
2020-02-20 7:26 ` [PATCH 1/8] bootconfig: Set CONFIG_BOOT_CONFIG=n by default Masami Hiramatsu
@ 2020-02-20 8:16 ` Geert Uytterhoeven
2020-02-20 8:55 ` Masami Hiramatsu
0 siblings, 1 reply; 11+ messages in thread
From: Geert Uytterhoeven @ 2020-02-20 8:16 UTC (permalink / raw)
To: Masami Hiramatsu
Cc: Steven Rostedt, Borislav Petkov, LKML, Ingo Molnar, Andrew Morton,
Peter Zijlstra
Hi Hiramatsu-san,
On Thu, Feb 20, 2020 at 8:26 AM Masami Hiramatsu <mhiramat@kernel.org> wrote:
> Set CONFIG_BOOT_CONFIG=n by default. This also warns
> user if CONFIG_BOOT_CONFIG=n but "bootconfig" is given
> in the kernel command line.
>
> Suggested-by: Steven Rostedt <rostedt@goodmis.org>
> Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org>
Thanks for your patch!
> --- a/init/Kconfig
> +++ b/init/Kconfig
> @@ -1218,7 +1218,7 @@ endif
> config BOOT_CONFIG
> bool "Boot config support"
> depends on BLK_DEV_INITRD
> - default y
> + default n
"default n" is the default, so you can just drop the line.
> help
> Extra boot config allows system admin to pass a config file as
> complemental extension of kernel cmdline when booting.
> diff --git a/init/main.c b/init/main.c
> index 59248717c925..680ff7123705 100644
> --- a/init/main.c
> +++ b/init/main.c
> @@ -418,6 +418,14 @@ static void __init setup_boot_config(const char *cmdline)
> }
> #else
> #define setup_boot_config(cmdline) do { } while (0)
> +
> +static int __init warn_bootconfig(char *str)
> +{
> + pr_err("WARNING: 'bootconfig' found on the kernel command line but CONFIG_BOOTCONFIG is not set.\n");
pr_warn()?
> + return 0;
> +}
> +early_param("bootconfig", warn_bootconfig);
> +
> #endif
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [PATCH 1/8] bootconfig: Set CONFIG_BOOT_CONFIG=n by default
2020-02-20 8:16 ` Geert Uytterhoeven
@ 2020-02-20 8:55 ` Masami Hiramatsu
0 siblings, 0 replies; 11+ messages in thread
From: Masami Hiramatsu @ 2020-02-20 8:55 UTC (permalink / raw)
To: Geert Uytterhoeven
Cc: Steven Rostedt, Borislav Petkov, LKML, Ingo Molnar, Andrew Morton,
Peter Zijlstra
On Thu, 20 Feb 2020 09:16:56 +0100
Geert Uytterhoeven <geert@linux-m68k.org> wrote:
> Hi Hiramatsu-san,
>
> On Thu, Feb 20, 2020 at 8:26 AM Masami Hiramatsu <mhiramat@kernel.org> wrote:
> > Set CONFIG_BOOT_CONFIG=n by default. This also warns
> > user if CONFIG_BOOT_CONFIG=n but "bootconfig" is given
> > in the kernel command line.
> >
> > Suggested-by: Steven Rostedt <rostedt@goodmis.org>
> > Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org>
>
> Thanks for your patch!
>
> > --- a/init/Kconfig
> > +++ b/init/Kconfig
> > @@ -1218,7 +1218,7 @@ endif
> > config BOOT_CONFIG
> > bool "Boot config support"
> > depends on BLK_DEV_INITRD
> > - default y
> > + default n
>
> "default n" is the default, so you can just drop the line.
OK.
>
> > help
> > Extra boot config allows system admin to pass a config file as
> > complemental extension of kernel cmdline when booting.
> > diff --git a/init/main.c b/init/main.c
> > index 59248717c925..680ff7123705 100644
> > --- a/init/main.c
> > +++ b/init/main.c
> > @@ -418,6 +418,14 @@ static void __init setup_boot_config(const char *cmdline)
> > }
> > #else
> > #define setup_boot_config(cmdline) do { } while (0)
> > +
> > +static int __init warn_bootconfig(char *str)
> > +{
> > + pr_err("WARNING: 'bootconfig' found on the kernel command line but CONFIG_BOOTCONFIG is not set.\n");
>
> pr_warn()?
Yeah, agreed.
OK, I'll update the patch.
Thank you!
>
> > + return 0;
> > +}
> > +early_param("bootconfig", warn_bootconfig);
> > +
> > #endif
>
> Gr{oetje,eeting}s,
>
> Geert
>
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
>
> In personal conversations with technical people, I call myself a hacker. But
> when I'm talking to journalists I just say "programmer" or something like that.
> -- Linus Torvalds
--
Masami Hiramatsu <mhiramat@kernel.org>
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH 2/8] bootconfig: Add bootconfig magic word for indicating bootconfig explicitly
2020-02-20 7:26 [PATCH 0/8] bootconfig: Update for the recent reports Masami Hiramatsu
2020-02-20 7:26 ` [PATCH 1/8] bootconfig: Set CONFIG_BOOT_CONFIG=n by default Masami Hiramatsu
@ 2020-02-20 7:26 ` Masami Hiramatsu
2020-02-20 7:26 ` [PATCH 3/8] tools/bootconfig: Remove unneeded error message silencer Masami Hiramatsu
` (5 subsequent siblings)
7 siblings, 0 replies; 11+ messages in thread
From: Masami Hiramatsu @ 2020-02-20 7:26 UTC (permalink / raw)
To: Steven Rostedt
Cc: Geert Uytterhoeven, Borislav Petkov, LKML, Ingo Molnar,
Andrew Morton, Masami Hiramatsu, Peter Zijlstra
Add bootconfig magic word to the end of bootconfig on initrd
image for indicating explicitly the bootconfig is there.
Also tools/bootconfig treats wrong size or wrong checksum or
parse error as an error, because if there is a bootconfig magic
word, there must be a bootconfig.
The bootconfig magic word is "#BOOTCONFIG\n", 12 bytes word.
Thus the block image of the initrd file with bootconfig is
as follows.
[Initrd][bootconfig][size][csum][#BOOTCONFIG\n]
Suggested-by: Steven Rostedt <rostedt@goodmis.org>
Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org>
---
Documentation/admin-guide/bootconfig.rst | 10 +++++--
include/linux/bootconfig.h | 3 ++
init/main.c | 6 +++-
tools/bootconfig/main.c | 43 ++++++++++++++++++++++--------
tools/bootconfig/test-bootconfig.sh | 2 +
5 files changed, 48 insertions(+), 16 deletions(-)
diff --git a/Documentation/admin-guide/bootconfig.rst b/Documentation/admin-guide/bootconfig.rst
index b342a6796392..5e7609936507 100644
--- a/Documentation/admin-guide/bootconfig.rst
+++ b/Documentation/admin-guide/bootconfig.rst
@@ -102,9 +102,13 @@ Boot Kernel With a Boot Config
==============================
Since the boot configuration file is loaded with initrd, it will be added
-to the end of the initrd (initramfs) image file. The Linux kernel decodes
-the last part of the initrd image in memory to get the boot configuration
-data.
+to the end of the initrd (initramfs) image file with size, checksum and
+12-byte magic word as below.
+
+[initrd][bootconfig][size(u32)][checksum(u32)][#BOOTCONFIG\n]
+
+The Linux kernel decodes the last part of the initrd image in memory to
+get the boot configuration data.
Because of this "piggyback" method, there is no need to change or
update the boot loader and the kernel image itself.
diff --git a/include/linux/bootconfig.h b/include/linux/bootconfig.h
index 7e18c939663e..d11e183fcb54 100644
--- a/include/linux/bootconfig.h
+++ b/include/linux/bootconfig.h
@@ -10,6 +10,9 @@
#include <linux/kernel.h>
#include <linux/types.h>
+#define BOOTCONFIG_MAGIC "#BOOTCONFIG\n"
+#define BOOTCONFIG_MAGIC_LEN 12
+
/* XBC tree node */
struct xbc_node {
u16 next;
diff --git a/init/main.c b/init/main.c
index 680ff7123705..671dd355fe9b 100644
--- a/init/main.c
+++ b/init/main.c
@@ -374,7 +374,11 @@ static void __init setup_boot_config(const char *cmdline)
if (!initrd_end)
goto not_found;
- hdr = (u32 *)(initrd_end - 8);
+ data = (char *)initrd_end - BOOTCONFIG_MAGIC_LEN;
+ if (memcmp(data, BOOTCONFIG_MAGIC, BOOTCONFIG_MAGIC_LEN))
+ goto not_found;
+
+ hdr = (u32 *)(data - 8);
size = hdr[0];
csum = hdr[1];
diff --git a/tools/bootconfig/main.c b/tools/bootconfig/main.c
index e18eeb070562..742271f019a9 100644
--- a/tools/bootconfig/main.c
+++ b/tools/bootconfig/main.c
@@ -131,15 +131,26 @@ int load_xbc_from_initrd(int fd, char **buf)
struct stat stat;
int ret;
u32 size = 0, csum = 0, rcsum;
+ char magic[BOOTCONFIG_MAGIC_LEN];
ret = fstat(fd, &stat);
if (ret < 0)
return -errno;
- if (stat.st_size < 8)
+ if (stat.st_size < 8 + BOOTCONFIG_MAGIC_LEN)
return 0;
- if (lseek(fd, -8, SEEK_END) < 0) {
+ if (lseek(fd, -BOOTCONFIG_MAGIC_LEN, SEEK_END) < 0) {
+ pr_err("Failed to lseek: %d\n", -errno);
+ return -errno;
+ }
+ if (read(fd, magic, BOOTCONFIG_MAGIC_LEN) < 0)
+ return -errno;
+ /* Check the bootconfig magic bytes */
+ if (memcmp(magic, BOOTCONFIG_MAGIC, BOOTCONFIG_MAGIC_LEN) != 0)
+ return 0;
+
+ if (lseek(fd, -(8 + BOOTCONFIG_MAGIC_LEN), SEEK_END) < 0) {
pr_err("Failed to lseek: %d\n", -errno);
return -errno;
}
@@ -150,11 +161,14 @@ int load_xbc_from_initrd(int fd, char **buf)
if (read(fd, &csum, sizeof(u32)) < 0)
return -errno;
- /* Wrong size, maybe no boot config here */
- if (stat.st_size < size + 8)
- return 0;
+ /* Wrong size error */
+ if (stat.st_size < size + 8 + BOOTCONFIG_MAGIC_LEN) {
+ pr_err("bootconfig size is too big\n");
+ return -E2BIG;
+ }
- if (lseek(fd, stat.st_size - 8 - size, SEEK_SET) < 0) {
+ if (lseek(fd, stat.st_size - (size + 8 + BOOTCONFIG_MAGIC_LEN),
+ SEEK_SET) < 0) {
pr_err("Failed to lseek: %d\n", -errno);
return -errno;
}
@@ -163,17 +177,17 @@ int load_xbc_from_initrd(int fd, char **buf)
if (ret < 0)
return ret;
- /* Wrong Checksum, maybe no boot config here */
+ /* Wrong Checksum */
rcsum = checksum((unsigned char *)*buf, size);
if (csum != rcsum) {
pr_err("checksum error: %d != %d\n", csum, rcsum);
- return 0;
+ return -EINVAL;
}
ret = xbc_init(*buf);
- /* Wrong data, maybe no boot config here */
+ /* Wrong data */
if (ret < 0)
- return 0;
+ return ret;
return size;
}
@@ -226,7 +240,8 @@ int delete_xbc(const char *path)
} else if (size > 0) {
ret = fstat(fd, &stat);
if (!ret)
- ret = ftruncate(fd, stat.st_size - size - 8);
+ ret = ftruncate(fd, stat.st_size
+ - size - 8 - BOOTCONFIG_MAGIC_LEN);
if (ret)
ret = -errno;
} /* Ignore if there is no boot config in initrd */
@@ -295,6 +310,12 @@ int apply_xbc(const char *path, const char *xbc_path)
pr_err("Failed to apply a boot config: %d\n", ret);
return ret;
}
+ /* Write a magic word of the bootconfig */
+ ret = write(fd, BOOTCONFIG_MAGIC, BOOTCONFIG_MAGIC_LEN);
+ if (ret < 0) {
+ pr_err("Failed to apply a boot config magic: %d\n", ret);
+ return ret;
+ }
close(fd);
free(data);
diff --git a/tools/bootconfig/test-bootconfig.sh b/tools/bootconfig/test-bootconfig.sh
index 1de06de328e2..adafb7c50940 100755
--- a/tools/bootconfig/test-bootconfig.sh
+++ b/tools/bootconfig/test-bootconfig.sh
@@ -49,7 +49,7 @@ xpass $BOOTCONF -a $TEMPCONF $INITRD
new_size=$(stat -c %s $INITRD)
echo "File size check"
-xpass test $new_size -eq $(expr $bconf_size + $initrd_size + 9)
+xpass test $new_size -eq $(expr $bconf_size + $initrd_size + 9 + 12)
echo "Apply command repeat test"
xpass $BOOTCONF -a $TEMPCONF $INITRD
^ permalink raw reply related [flat|nested] 11+ messages in thread* [PATCH 3/8] tools/bootconfig: Remove unneeded error message silencer
2020-02-20 7:26 [PATCH 0/8] bootconfig: Update for the recent reports Masami Hiramatsu
2020-02-20 7:26 ` [PATCH 1/8] bootconfig: Set CONFIG_BOOT_CONFIG=n by default Masami Hiramatsu
2020-02-20 7:26 ` [PATCH 2/8] bootconfig: Add bootconfig magic word for indicating bootconfig explicitly Masami Hiramatsu
@ 2020-02-20 7:26 ` Masami Hiramatsu
2020-02-20 7:27 ` [PATCH 4/8] bootconfig: Remove unneeded checksum Masami Hiramatsu
` (4 subsequent siblings)
7 siblings, 0 replies; 11+ messages in thread
From: Masami Hiramatsu @ 2020-02-20 7:26 UTC (permalink / raw)
To: Steven Rostedt
Cc: Geert Uytterhoeven, Borislav Petkov, LKML, Ingo Molnar,
Andrew Morton, Masami Hiramatsu, Peter Zijlstra
Remove error message silent knob, we don't need it anymore
because we can check if there is a bootconfig by checking
the magic word.
If there is a magic word, but failed to load a bootconfig
from initrd, there is a real problem.
Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org>
---
tools/bootconfig/include/linux/printk.h | 5 +----
tools/bootconfig/main.c | 8 --------
2 files changed, 1 insertion(+), 12 deletions(-)
diff --git a/tools/bootconfig/include/linux/printk.h b/tools/bootconfig/include/linux/printk.h
index e978a63d3222..036e667596eb 100644
--- a/tools/bootconfig/include/linux/printk.h
+++ b/tools/bootconfig/include/linux/printk.h
@@ -4,10 +4,7 @@
#include <stdio.h>
-/* controllable printf */
-extern int pr_output;
-#define printk(fmt, ...) \
- (pr_output ? printf(fmt, ##__VA_ARGS__) : 0)
+#define printk(fmt, ...) printf(fmt, ##__VA_ARGS__)
#define pr_err printk
#define pr_warn printk
diff --git a/tools/bootconfig/main.c b/tools/bootconfig/main.c
index 742271f019a9..a9b97814d1a9 100644
--- a/tools/bootconfig/main.c
+++ b/tools/bootconfig/main.c
@@ -14,8 +14,6 @@
#include <linux/kernel.h>
#include <linux/bootconfig.h>
-int pr_output = 1;
-
static int xbc_show_array(struct xbc_node *node)
{
const char *val;
@@ -227,13 +225,7 @@ int delete_xbc(const char *path)
return -errno;
}
- /*
- * Suppress error messages in xbc_init() because it can be just a
- * data which concidentally matches the size and checksum footer.
- */
- pr_output = 0;
size = load_xbc_from_initrd(fd, &buf);
- pr_output = 1;
if (size < 0) {
ret = size;
pr_err("Failed to load a boot config from initrd: %d\n", ret);
^ permalink raw reply related [flat|nested] 11+ messages in thread* [PATCH 4/8] bootconfig: Remove unneeded checksum
2020-02-20 7:26 [PATCH 0/8] bootconfig: Update for the recent reports Masami Hiramatsu
` (2 preceding siblings ...)
2020-02-20 7:26 ` [PATCH 3/8] tools/bootconfig: Remove unneeded error message silencer Masami Hiramatsu
@ 2020-02-20 7:27 ` Masami Hiramatsu
2020-02-20 7:27 ` [PATCH 5/8] bootconfig: Reject subkey and value on same parent key Masami Hiramatsu
` (3 subsequent siblings)
7 siblings, 0 replies; 11+ messages in thread
From: Masami Hiramatsu @ 2020-02-20 7:27 UTC (permalink / raw)
To: Steven Rostedt
Cc: Geert Uytterhoeven, Borislav Petkov, LKML, Ingo Molnar,
Andrew Morton, Masami Hiramatsu, Peter Zijlstra
Remove checksum of bootconfig because we already use a magic
word to identify bootconfig. This checksum was used for
checking whether there is a bootconfig at the end of initrd
or not. Since we have a bootconfig magic word to identify
the bootconfig data, we do not this checksum anymore.
Thus the block image of the initrd file with bootconfig is
as follows.
[initrd][bootconfig][size(u32)][#BOOTCONFIG\n]
Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org>
---
Documentation/admin-guide/bootconfig.rst | 6 ++--
init/main.c | 20 +-------------
tools/bootconfig/main.c | 42 ++++++------------------------
tools/bootconfig/test-bootconfig.sh | 2 +
4 files changed, 15 insertions(+), 55 deletions(-)
diff --git a/Documentation/admin-guide/bootconfig.rst b/Documentation/admin-guide/bootconfig.rst
index 5e7609936507..48675052c963 100644
--- a/Documentation/admin-guide/bootconfig.rst
+++ b/Documentation/admin-guide/bootconfig.rst
@@ -102,10 +102,10 @@ Boot Kernel With a Boot Config
==============================
Since the boot configuration file is loaded with initrd, it will be added
-to the end of the initrd (initramfs) image file with size, checksum and
-12-byte magic word as below.
+to the end of the initrd (initramfs) image file with 4-byte size and 12-byte
+magic word as below.
-[initrd][bootconfig][size(u32)][checksum(u32)][#BOOTCONFIG\n]
+[initrd][bootconfig][size][#BOOTCONFIG\n]
The Linux kernel decodes the last part of the initrd image in memory to
get the boot configuration data.
diff --git a/init/main.c b/init/main.c
index 671dd355fe9b..7c4ca805562a 100644
--- a/init/main.c
+++ b/init/main.c
@@ -335,16 +335,6 @@ static char * __init xbc_make_cmdline(const char *key)
return new_cmdline;
}
-u32 boot_config_checksum(unsigned char *p, u32 size)
-{
- u32 ret = 0;
-
- while (size--)
- ret += *p++;
-
- return ret;
-}
-
static int __init bootconfig_params(char *param, char *val,
const char *unused, void *arg)
{
@@ -359,7 +349,7 @@ static int __init bootconfig_params(char *param, char *val,
static void __init setup_boot_config(const char *cmdline)
{
static char tmp_cmdline[COMMAND_LINE_SIZE] __initdata;
- u32 size, csum;
+ u32 size;
char *data, *copy;
u32 *hdr;
int ret;
@@ -378,9 +368,8 @@ static void __init setup_boot_config(const char *cmdline)
if (memcmp(data, BOOTCONFIG_MAGIC, BOOTCONFIG_MAGIC_LEN))
goto not_found;
- hdr = (u32 *)(data - 8);
+ hdr = (u32 *)(data - sizeof(u32));
size = hdr[0];
- csum = hdr[1];
if (size >= XBC_DATA_MAX) {
pr_err("bootconfig size %d greater than max size %d\n",
@@ -392,11 +381,6 @@ static void __init setup_boot_config(const char *cmdline)
if ((unsigned long)data < initrd_start)
goto not_found;
- if (boot_config_checksum((unsigned char *)data, size) != csum) {
- pr_err("bootconfig checksum failed\n");
- return;
- }
-
copy = memblock_alloc(size + 1, SMP_CACHE_BYTES);
if (!copy) {
pr_err("Failed to allocate memory for bootconfig\n");
diff --git a/tools/bootconfig/main.c b/tools/bootconfig/main.c
index a9b97814d1a9..01acb86c6273 100644
--- a/tools/bootconfig/main.c
+++ b/tools/bootconfig/main.c
@@ -75,17 +75,6 @@ static void xbc_show_compact_tree(void)
}
}
-/* Simple real checksum */
-int checksum(unsigned char *buf, int len)
-{
- int i, sum = 0;
-
- for (i = 0; i < len; i++)
- sum += buf[i];
-
- return sum;
-}
-
#define PAGE_SIZE 4096
int load_xbc_fd(int fd, char **buf, int size)
@@ -128,14 +117,14 @@ int load_xbc_from_initrd(int fd, char **buf)
{
struct stat stat;
int ret;
- u32 size = 0, csum = 0, rcsum;
+ u32 size = 0;
char magic[BOOTCONFIG_MAGIC_LEN];
ret = fstat(fd, &stat);
if (ret < 0)
return -errno;
- if (stat.st_size < 8 + BOOTCONFIG_MAGIC_LEN)
+ if (stat.st_size < 4 + BOOTCONFIG_MAGIC_LEN)
return 0;
if (lseek(fd, -BOOTCONFIG_MAGIC_LEN, SEEK_END) < 0) {
@@ -148,7 +137,7 @@ int load_xbc_from_initrd(int fd, char **buf)
if (memcmp(magic, BOOTCONFIG_MAGIC, BOOTCONFIG_MAGIC_LEN) != 0)
return 0;
- if (lseek(fd, -(8 + BOOTCONFIG_MAGIC_LEN), SEEK_END) < 0) {
+ if (lseek(fd, -(4 + BOOTCONFIG_MAGIC_LEN), SEEK_END) < 0) {
pr_err("Failed to lseek: %d\n", -errno);
return -errno;
}
@@ -156,16 +145,13 @@ int load_xbc_from_initrd(int fd, char **buf)
if (read(fd, &size, sizeof(u32)) < 0)
return -errno;
- if (read(fd, &csum, sizeof(u32)) < 0)
- return -errno;
-
/* Wrong size error */
- if (stat.st_size < size + 8 + BOOTCONFIG_MAGIC_LEN) {
+ if (stat.st_size < size + 4 + BOOTCONFIG_MAGIC_LEN) {
pr_err("bootconfig size is too big\n");
return -E2BIG;
}
- if (lseek(fd, stat.st_size - (size + 8 + BOOTCONFIG_MAGIC_LEN),
+ if (lseek(fd, stat.st_size - (size + 4 + BOOTCONFIG_MAGIC_LEN),
SEEK_SET) < 0) {
pr_err("Failed to lseek: %d\n", -errno);
return -errno;
@@ -175,13 +161,6 @@ int load_xbc_from_initrd(int fd, char **buf)
if (ret < 0)
return ret;
- /* Wrong Checksum */
- rcsum = checksum((unsigned char *)*buf, size);
- if (csum != rcsum) {
- pr_err("checksum error: %d != %d\n", csum, rcsum);
- return -EINVAL;
- }
-
ret = xbc_init(*buf);
/* Wrong data */
if (ret < 0)
@@ -233,7 +212,7 @@ int delete_xbc(const char *path)
ret = fstat(fd, &stat);
if (!ret)
ret = ftruncate(fd, stat.st_size
- - size - 8 - BOOTCONFIG_MAGIC_LEN);
+ - size - 4 - BOOTCONFIG_MAGIC_LEN);
if (ret)
ret = -errno;
} /* Ignore if there is no boot config in initrd */
@@ -246,7 +225,7 @@ int delete_xbc(const char *path)
int apply_xbc(const char *path, const char *xbc_path)
{
- u32 size, csum;
+ u32 size;
char *buf, *data;
int ret, fd;
@@ -256,15 +235,13 @@ int apply_xbc(const char *path, const char *xbc_path)
return ret;
}
size = strlen(buf) + 1;
- csum = checksum((unsigned char *)buf, size);
/* Prepare xbc_path data */
- data = malloc(size + 8);
+ data = malloc(size + 4);
if (!data)
return -ENOMEM;
strcpy(data, buf);
*(u32 *)(data + size) = size;
- *(u32 *)(data + size + 4) = csum;
/* Check the data format */
ret = xbc_init(buf);
@@ -277,7 +254,6 @@ int apply_xbc(const char *path, const char *xbc_path)
printf("Apply %s to %s\n", xbc_path, path);
printf("\tNumber of nodes: %d\n", ret);
printf("\tSize: %u bytes\n", (unsigned int)size);
- printf("\tChecksum: %d\n", (unsigned int)csum);
/* TODO: Check the options by schema */
xbc_destroy_all();
@@ -297,7 +273,7 @@ int apply_xbc(const char *path, const char *xbc_path)
return fd;
}
/* TODO: Ensure the @path is initramfs/initrd image */
- ret = write(fd, data, size + 8);
+ ret = write(fd, data, size + 4);
if (ret < 0) {
pr_err("Failed to apply a boot config: %d\n", ret);
return ret;
diff --git a/tools/bootconfig/test-bootconfig.sh b/tools/bootconfig/test-bootconfig.sh
index adafb7c50940..c5965eff62c5 100755
--- a/tools/bootconfig/test-bootconfig.sh
+++ b/tools/bootconfig/test-bootconfig.sh
@@ -49,7 +49,7 @@ xpass $BOOTCONF -a $TEMPCONF $INITRD
new_size=$(stat -c %s $INITRD)
echo "File size check"
-xpass test $new_size -eq $(expr $bconf_size + $initrd_size + 9 + 12)
+xpass test $new_size -eq $(expr $bconf_size + $initrd_size + 5 + 12)
echo "Apply command repeat test"
xpass $BOOTCONF -a $TEMPCONF $INITRD
^ permalink raw reply related [flat|nested] 11+ messages in thread* [PATCH 5/8] bootconfig: Reject subkey and value on same parent key
2020-02-20 7:26 [PATCH 0/8] bootconfig: Update for the recent reports Masami Hiramatsu
` (3 preceding siblings ...)
2020-02-20 7:27 ` [PATCH 4/8] bootconfig: Remove unneeded checksum Masami Hiramatsu
@ 2020-02-20 7:27 ` Masami Hiramatsu
2020-02-20 7:27 ` [PATCH 6/8] bootconfig: Overwrite value on same key by default Masami Hiramatsu
` (2 subsequent siblings)
7 siblings, 0 replies; 11+ messages in thread
From: Masami Hiramatsu @ 2020-02-20 7:27 UTC (permalink / raw)
To: Steven Rostedt
Cc: Geert Uytterhoeven, Borislav Petkov, LKML, Ingo Molnar,
Andrew Morton, Masami Hiramatsu, Peter Zijlstra
Reject if a value node is mixed with subkey node on same
parent key node.
A value node can not co-exist with subkey node under some key
node, e.g.
key = value
key.subkey = another-value
This is not be allowed because bootconfig API is not designed
to handle such case.
Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org>
---
Documentation/admin-guide/bootconfig.rst | 7 +++++++
lib/bootconfig.c | 16 ++++++++++++----
tools/bootconfig/samples/bad-mixed-kv1.bconf | 3 +++
tools/bootconfig/samples/bad-mixed-kv2.bconf | 3 +++
4 files changed, 25 insertions(+), 4 deletions(-)
create mode 100644 tools/bootconfig/samples/bad-mixed-kv1.bconf
create mode 100644 tools/bootconfig/samples/bad-mixed-kv2.bconf
diff --git a/Documentation/admin-guide/bootconfig.rst b/Documentation/admin-guide/bootconfig.rst
index 48675052c963..4a106584eb21 100644
--- a/Documentation/admin-guide/bootconfig.rst
+++ b/Documentation/admin-guide/bootconfig.rst
@@ -62,6 +62,13 @@ Or more shorter, written as following::
In both styles, same key words are automatically merged when parsing it
at boot time. So you can append similar trees or key-values.
+Note that a sub-key and a value can not co-exist under a parent key.
+For example, following config is NOT allowed.::
+
+ foo = value1
+ foo.bar = value2 # !ERROR! subkey "bar" and value "value1" can NOT co-exist
+
+
Comments
--------
diff --git a/lib/bootconfig.c b/lib/bootconfig.c
index 3ea601a2eba5..54ac623ca781 100644
--- a/lib/bootconfig.c
+++ b/lib/bootconfig.c
@@ -533,7 +533,7 @@ struct xbc_node *find_match_node(struct xbc_node *node, char *k)
static int __init __xbc_add_key(char *k)
{
- struct xbc_node *node;
+ struct xbc_node *node, *child;
if (!xbc_valid_keyword(k))
return xbc_parse_error("Invalid keyword", k);
@@ -543,8 +543,12 @@ static int __init __xbc_add_key(char *k)
if (!last_parent) /* the first level */
node = find_match_node(xbc_nodes, k);
- else
- node = find_match_node(xbc_node_get_child(last_parent), k);
+ else {
+ child = xbc_node_get_child(last_parent);
+ if (child && xbc_node_is_value(child))
+ return xbc_parse_error("Subkey is mixed with value", k);
+ node = find_match_node(child, k);
+ }
if (node)
last_parent = node;
@@ -577,7 +581,7 @@ static int __init __xbc_parse_keys(char *k)
static int __init xbc_parse_kv(char **k, char *v)
{
struct xbc_node *prev_parent = last_parent;
- struct xbc_node *node;
+ struct xbc_node *node, *child;
char *next;
int c, ret;
@@ -585,6 +589,10 @@ static int __init xbc_parse_kv(char **k, char *v)
if (ret)
return ret;
+ child = xbc_node_get_child(last_parent);
+ if (child && xbc_node_is_key(child))
+ return xbc_parse_error("Value is mixed with subkey", v);
+
c = __xbc_parse_value(&v, &next);
if (c < 0)
return c;
diff --git a/tools/bootconfig/samples/bad-mixed-kv1.bconf b/tools/bootconfig/samples/bad-mixed-kv1.bconf
new file mode 100644
index 000000000000..1761547dd05c
--- /dev/null
+++ b/tools/bootconfig/samples/bad-mixed-kv1.bconf
@@ -0,0 +1,3 @@
+# value -> subkey pattern
+key = value
+key.subkey = another-value
diff --git a/tools/bootconfig/samples/bad-mixed-kv2.bconf b/tools/bootconfig/samples/bad-mixed-kv2.bconf
new file mode 100644
index 000000000000..6b32e0c3878c
--- /dev/null
+++ b/tools/bootconfig/samples/bad-mixed-kv2.bconf
@@ -0,0 +1,3 @@
+# subkey -> value pattern
+key.subkey = value
+key = another-value
^ permalink raw reply related [flat|nested] 11+ messages in thread* [PATCH 6/8] bootconfig: Overwrite value on same key by default
2020-02-20 7:26 [PATCH 0/8] bootconfig: Update for the recent reports Masami Hiramatsu
` (4 preceding siblings ...)
2020-02-20 7:27 ` [PATCH 5/8] bootconfig: Reject subkey and value on same parent key Masami Hiramatsu
@ 2020-02-20 7:27 ` Masami Hiramatsu
2020-02-20 7:27 ` [PATCH 7/8] bootconfig: Add append value operator support Masami Hiramatsu
2020-02-20 7:27 ` [PATCH 8/8] bootconfig: Print array as multiple commands for legacy command line Masami Hiramatsu
7 siblings, 0 replies; 11+ messages in thread
From: Masami Hiramatsu @ 2020-02-20 7:27 UTC (permalink / raw)
To: Steven Rostedt
Cc: Geert Uytterhoeven, Borislav Petkov, LKML, Ingo Molnar,
Andrew Morton, Masami Hiramatsu, Peter Zijlstra
Currently, bootconfig does not overwrite existing value
on same key, but add new value to the tail of an array.
But this looks a bit confusing because similar syntax
configuration always overwrite the value by default.
This changes the behavior to overwrite value on same key.
For example, if there are 2 entries
key = value
...
key = value2
Then, the key is updated as below.
key = value2
Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org>
---
Documentation/admin-guide/bootconfig.rst | 12 ++++++++++++
lib/bootconfig.c | 18 ++++++++++++++----
tools/bootconfig/test-bootconfig.sh | 16 ++++++++++++++--
3 files changed, 40 insertions(+), 6 deletions(-)
diff --git a/Documentation/admin-guide/bootconfig.rst b/Documentation/admin-guide/bootconfig.rst
index 4a106584eb21..0c18e3f540e6 100644
--- a/Documentation/admin-guide/bootconfig.rst
+++ b/Documentation/admin-guide/bootconfig.rst
@@ -62,6 +62,18 @@ Or more shorter, written as following::
In both styles, same key words are automatically merged when parsing it
at boot time. So you can append similar trees or key-values.
+Same-key Values
+---------------
+
+If two or more values or arraies share a same-key, the latter one remains.
+For example,::
+
+ foo = bar, baz
+ foo = qux
+
+In this case ``bar`` and ``baz`` is overwritten by ``qux``, and ``foo = qux``
+remains.
+
Note that a sub-key and a value can not co-exist under a parent key.
For example, following config is NOT allowed.::
diff --git a/lib/bootconfig.c b/lib/bootconfig.c
index 54ac623ca781..9a094162ea3e 100644
--- a/lib/bootconfig.c
+++ b/lib/bootconfig.c
@@ -578,10 +578,18 @@ static int __init __xbc_parse_keys(char *k)
return __xbc_add_key(k);
}
+static void xbc_node_overwrite(struct xbc_node *node, char *v)
+{
+ unsigned long offset = v - xbc_data;
+
+ node->data = (u16)offset | XBC_VALUE;
+ node->next = 0;
+}
+
static int __init xbc_parse_kv(char **k, char *v)
{
struct xbc_node *prev_parent = last_parent;
- struct xbc_node *node, *child;
+ struct xbc_node *child;
char *next;
int c, ret;
@@ -597,9 +605,11 @@ static int __init xbc_parse_kv(char **k, char *v)
if (c < 0)
return c;
- node = xbc_add_sibling(v, XBC_VALUE);
- if (!node)
- return -ENOMEM;
+ if (!child) {
+ if (!xbc_add_sibling(v, XBC_VALUE))
+ return -ENOMEM;
+ } else
+ xbc_node_overwrite(child, v);
if (c == ',') { /* Array */
c = xbc_parse_array(&next);
diff --git a/tools/bootconfig/test-bootconfig.sh b/tools/bootconfig/test-bootconfig.sh
index c5965eff62c5..f6948790c693 100755
--- a/tools/bootconfig/test-bootconfig.sh
+++ b/tools/bootconfig/test-bootconfig.sh
@@ -9,7 +9,7 @@ TEMPCONF=`mktemp temp-XXXX.bconf`
NG=0
cleanup() {
- rm -f $INITRD $TEMPCONF
+ rm -f $INITRD $TEMPCONF $OUTFILE
exit $NG
}
@@ -71,7 +71,6 @@ printf " \0\0\0 \0\0\0" >> $INITRD
$BOOTCONF -a $TEMPCONF $INITRD > $OUTFILE 2>&1
xfail grep -i "failed" $OUTFILE
xfail grep -i "error" $OUTFILE
-rm $OUTFILE
echo "Max node number check"
@@ -96,6 +95,19 @@ truncate -s 32764 $TEMPCONF
echo "\"" >> $TEMPCONF # add 2 bytes + terminal ('\"\n\0')
xpass $BOOTCONF -a $TEMPCONF $INITRD
+echo "Overwrite same-key values"
+cat > $TEMPCONF << EOF
+key = bar, baz
+key = qux
+EOF
+echo > $INITRD
+
+xpass $BOOTCONF -a $TEMPCONF $INITRD
+$BOOTCONF $INITRD > $OUTFILE
+xfail grep -q "bar" $OUTFILE
+xfail grep -q "baz" $OUTFILE
+xpass grep -q "qux" $OUTFILE
+
echo "=== expected failure cases ==="
for i in samples/bad-* ; do
xfail $BOOTCONF -a $i $INITRD
^ permalink raw reply related [flat|nested] 11+ messages in thread* [PATCH 7/8] bootconfig: Add append value operator support
2020-02-20 7:26 [PATCH 0/8] bootconfig: Update for the recent reports Masami Hiramatsu
` (5 preceding siblings ...)
2020-02-20 7:27 ` [PATCH 6/8] bootconfig: Overwrite value on same key by default Masami Hiramatsu
@ 2020-02-20 7:27 ` Masami Hiramatsu
2020-02-20 7:27 ` [PATCH 8/8] bootconfig: Print array as multiple commands for legacy command line Masami Hiramatsu
7 siblings, 0 replies; 11+ messages in thread
From: Masami Hiramatsu @ 2020-02-20 7:27 UTC (permalink / raw)
To: Steven Rostedt
Cc: Geert Uytterhoeven, Borislav Petkov, LKML, Ingo Molnar,
Andrew Morton, Masami Hiramatsu, Peter Zijlstra
Add append value operator "+=" support to bootconfig syntax.
With this operator, user can add new value to the key as
an entry of array instead of overwriting.
For example,
foo = bar
...
foo += baz
Then the key "foo" has "bar" and "baz" values as an array.
Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org>
---
Documentation/admin-guide/bootconfig.rst | 8 ++++++++
lib/bootconfig.c | 14 ++++++++++----
tools/bootconfig/test-bootconfig.sh | 13 +++++++++++++
3 files changed, 31 insertions(+), 4 deletions(-)
diff --git a/Documentation/admin-guide/bootconfig.rst b/Documentation/admin-guide/bootconfig.rst
index 0c18e3f540e6..abe2432b8eec 100644
--- a/Documentation/admin-guide/bootconfig.rst
+++ b/Documentation/admin-guide/bootconfig.rst
@@ -74,6 +74,14 @@ For example,::
In this case ``bar`` and ``baz`` is overwritten by ``qux``, and ``foo = qux``
remains.
+If you want to append the value to existing key as an array member,
+you can use ``+=`` operator. For example::
+
+ foo = bar, baz
+ foo += qux
+
+In this case, the key ``foo`` has ``bar``, ``baz`` and ``qux``.
+
Note that a sub-key and a value can not co-exist under a parent key.
For example, following config is NOT allowed.::
diff --git a/lib/bootconfig.c b/lib/bootconfig.c
index 9a094162ea3e..4afc768489cd 100644
--- a/lib/bootconfig.c
+++ b/lib/bootconfig.c
@@ -586,7 +586,7 @@ static void xbc_node_overwrite(struct xbc_node *node, char *v)
node->next = 0;
}
-static int __init xbc_parse_kv(char **k, char *v)
+static int __init xbc_parse_kv(char **k, char *v, int op)
{
struct xbc_node *prev_parent = last_parent;
struct xbc_node *child;
@@ -605,7 +605,7 @@ static int __init xbc_parse_kv(char **k, char *v)
if (c < 0)
return c;
- if (!child) {
+ if (op == '+' || !child) {
if (!xbc_add_sibling(v, XBC_VALUE))
return -ENOMEM;
} else
@@ -781,7 +781,7 @@ int __init xbc_init(char *buf)
p = buf;
do {
- q = strpbrk(p, "{}=;\n#");
+ q = strpbrk(p, "{}=+;\n#");
if (!q) {
p = skip_spaces(p);
if (*p != '\0')
@@ -792,8 +792,14 @@ int __init xbc_init(char *buf)
c = *q;
*q++ = '\0';
switch (c) {
+ case '+':
+ if (*q++ != '=') {
+ ret = xbc_parse_error("Wrong + operator", q - 2);
+ break;
+ }
+ /* Fall through */
case '=':
- ret = xbc_parse_kv(&p, q);
+ ret = xbc_parse_kv(&p, q, c);
break;
case '{':
ret = xbc_open_brace(&p, q);
diff --git a/tools/bootconfig/test-bootconfig.sh b/tools/bootconfig/test-bootconfig.sh
index f6948790c693..04f65548ca1e 100755
--- a/tools/bootconfig/test-bootconfig.sh
+++ b/tools/bootconfig/test-bootconfig.sh
@@ -108,6 +108,19 @@ xfail grep -q "bar" $OUTFILE
xfail grep -q "baz" $OUTFILE
xpass grep -q "qux" $OUTFILE
+echo "Adding same-key values"
+cat > $TEMPCONF << EOF
+key = bar, baz
+key += qux
+EOF
+echo > $INITRD
+
+xpass $BOOTCONF -a $TEMPCONF $INITRD
+$BOOTCONF $INITRD > $OUTFILE
+xpass grep -q "bar" $OUTFILE
+xpass grep -q "baz" $OUTFILE
+xpass grep -q "qux" $OUTFILE
+
echo "=== expected failure cases ==="
for i in samples/bad-* ; do
xfail $BOOTCONF -a $i $INITRD
^ permalink raw reply related [flat|nested] 11+ messages in thread* [PATCH 8/8] bootconfig: Print array as multiple commands for legacy command line
2020-02-20 7:26 [PATCH 0/8] bootconfig: Update for the recent reports Masami Hiramatsu
` (6 preceding siblings ...)
2020-02-20 7:27 ` [PATCH 7/8] bootconfig: Add append value operator support Masami Hiramatsu
@ 2020-02-20 7:27 ` Masami Hiramatsu
7 siblings, 0 replies; 11+ messages in thread
From: Masami Hiramatsu @ 2020-02-20 7:27 UTC (permalink / raw)
To: Steven Rostedt
Cc: Geert Uytterhoeven, Borislav Petkov, LKML, Ingo Molnar,
Andrew Morton, Masami Hiramatsu, Peter Zijlstra
Print arraied values as multiple same options for legacy
kernel command line. With this rule, if the "kernel.*" and
"init.*" array entries in bootconfig are printed out as
multiple same options, e.g.
kernel {
console = "ttyS0,115200"
console += "tty0"
}
will be correctly converted to
console="ttyS0,115200" console="tty0"
in the kernel command line.
Reported-by: Borislav Petkov <bp@alien8.de>
Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org>
---
init/main.c | 22 ++++++++--------------
1 file changed, 8 insertions(+), 14 deletions(-)
diff --git a/init/main.c b/init/main.c
index 7c4ca805562a..606c702f5fc8 100644
--- a/init/main.c
+++ b/init/main.c
@@ -268,7 +268,6 @@ static int __init xbc_snprint_cmdline(char *buf, size_t size,
{
struct xbc_node *knode, *vnode;
char *end = buf + size;
- char c = '\"';
const char *val;
int ret;
@@ -279,25 +278,20 @@ static int __init xbc_snprint_cmdline(char *buf, size_t size,
return ret;
vnode = xbc_node_get_child(knode);
- ret = snprintf(buf, rest(buf, end), "%s%c", xbc_namebuf,
- vnode ? '=' : ' ');
- if (ret < 0)
- return ret;
- buf += ret;
- if (!vnode)
+ if (!vnode) {
+ ret = snprintf(buf, rest(buf, end), "%s ", xbc_namebuf);
+ if (ret < 0)
+ return ret;
+ buf += ret;
continue;
-
- c = '\"';
+ }
xbc_array_for_each_value(vnode, val) {
- ret = snprintf(buf, rest(buf, end), "%c%s", c, val);
+ ret = snprintf(buf, rest(buf, end), "%s=\"%s\" ",
+ xbc_namebuf, val);
if (ret < 0)
return ret;
buf += ret;
- c = ',';
}
- if (rest(buf, end) > 2)
- strcpy(buf, "\" ");
- buf += 2;
}
return buf - (end - size);
^ permalink raw reply related [flat|nested] 11+ messages in thread