Re: [RFC PATCH v1 2/2] printk: external log buffer (CONFIG_LOGBUFFER)
On 10/5/2016 2:48 PM, Petr Mladek wrote: > On Tue 2016-10-04 12:55:35, Sean Hudson wrote: >> On 10/4/2016 6:27 AM, Petr Mladek wrote: >>> On Fri 2016-09-30 23:06:49, Sean Hudson wrote: >>>> On 9/30/2016 7:39 AM, Petr Mladek wrote: >>>>> >> Note, there is a small window of time during early boot in which the >> kernel puts entries into the default location, a staticlly declared >> char[]. These entries are appended to the external location once >> setup_ext_logbuff() is called from init/main.c. (I'd love to close that >> gap entirely, but all solutions I've tried involve arch specific code, >> which I'm trying to avoid.) > > IMHO, a copying of few messages is fine. Well, we need to make sure > that they are not printed twice on the console. I happen to agree since this is a debug feature. It would be nice to avoid it, but every strategy that I've considered makes the code less portable. >>>>> How protected will the address of the shared buffer? Would not >>>>> it open a security hole? We need to make sure that non-privileged >>>>> user or non-trusted application must not read sensitive data >>>>> from the log or break the structure of the log by writing >>>>> invalid data. >>>> >>>> There is no notion of protection of the address. However, additional >>>> checking could be added. Currently, there is potential for corrupt log >>>> entries from the bootloader causing the kernel to crash. >>>> I can't think of a way to prevent that from happening. >>> >>> IMHO, the only way is to revisit all locations when the log buffer >>> is accessed and add all the needed consistency checks. We must make >>> sure that we do not access outside of the buffer. Also the log_*_seq >>> numbers are supposed to grow linearly... >> >> Adding an explicit walk of the external log entries should be trivial. >> That will only validate that the passed in log entries are consistent >> internally. I will be submitting a v2 patch shortly that fixes an error >> that kernel-ci caught. (if CONFIG_PRINTK is disabled, compilation fails) >> I'll see about adding that check at the same time. > > The question is if a one time walk might find all potential problems. > We might want to add checks into the existing code. For example, > log_from_idx() is rather sensitive but it does not check if > all the values are sane. Checking for every possible problem isn't practical and would adversely impact the performance of the logging code. In the case of the function you point out, the patch series doesn't change the logic, rather it replaces the structure being used. >>>> Also, as you point out, a bootloader could read log contents from >>>> the shared region. Of course, in order for either to happen, >>>> the bootloader would already be compromised and I'm not sure that >>>> reading log entries or crashing the kernel is such a big consideration. >>> >>> The log might contain addresses of some kernel structures, for example >>> in some error messages. For this reason, only a privileged users are >>> able to read it. We must make sure that they will not get accessible >>> for a non-privileged user, for example via an address to the shared >>> buffer in the bootloader config. >> >> Ahhh, if I am understanding correctly, your concern is that an >> unprivileged program in user space would be able to gain access directly >> to the log entries via the external buffer location. I haven't tested >> that scenario, so I'm not sure if the memory reservation via the DT >> protects that memory from user space. I'll have to check on that. In >> the meantime, if you know of standard ways to protect memory from user >> space access, please let me know. > > I am not expert. I think that the main trick on the memory > management level is that the userspace and kernel have different > address spaces. IMHO, the pages that are used by the kernel log buffer > cannot be addressed from userspace. In fact, each user have only very > limited number of pages that are mapped to his address space. > > I do not know how this works with the persistent memory. > > In each case, we need to make sure that the address of the > shared buffer is not readable in the booloaded config file > for a normal user. I do not agree with that last point. If you cannot trust the bootloader then the ability of a userspace program to read log contents seems to be the least of your problems. >>> BTW: You did not answered the question about how the bootloader would >>
Re: [RFC PATCH v1 2/2] printk: external log buffer (CONFIG_LOGBUFFER)
On 10/5/2016 2:48 PM, Petr Mladek wrote: > On Tue 2016-10-04 12:55:35, Sean Hudson wrote: >> On 10/4/2016 6:27 AM, Petr Mladek wrote: >>> On Fri 2016-09-30 23:06:49, Sean Hudson wrote: >>>> On 9/30/2016 7:39 AM, Petr Mladek wrote: >>>>> >> Note, there is a small window of time during early boot in which the >> kernel puts entries into the default location, a staticlly declared >> char[]. These entries are appended to the external location once >> setup_ext_logbuff() is called from init/main.c. (I'd love to close that >> gap entirely, but all solutions I've tried involve arch specific code, >> which I'm trying to avoid.) > > IMHO, a copying of few messages is fine. Well, we need to make sure > that they are not printed twice on the console. I happen to agree since this is a debug feature. It would be nice to avoid it, but every strategy that I've considered makes the code less portable. >>>>> How protected will the address of the shared buffer? Would not >>>>> it open a security hole? We need to make sure that non-privileged >>>>> user or non-trusted application must not read sensitive data >>>>> from the log or break the structure of the log by writing >>>>> invalid data. >>>> >>>> There is no notion of protection of the address. However, additional >>>> checking could be added. Currently, there is potential for corrupt log >>>> entries from the bootloader causing the kernel to crash. >>>> I can't think of a way to prevent that from happening. >>> >>> IMHO, the only way is to revisit all locations when the log buffer >>> is accessed and add all the needed consistency checks. We must make >>> sure that we do not access outside of the buffer. Also the log_*_seq >>> numbers are supposed to grow linearly... >> >> Adding an explicit walk of the external log entries should be trivial. >> That will only validate that the passed in log entries are consistent >> internally. I will be submitting a v2 patch shortly that fixes an error >> that kernel-ci caught. (if CONFIG_PRINTK is disabled, compilation fails) >> I'll see about adding that check at the same time. > > The question is if a one time walk might find all potential problems. > We might want to add checks into the existing code. For example, > log_from_idx() is rather sensitive but it does not check if > all the values are sane. Checking for every possible problem isn't practical and would adversely impact the performance of the logging code. In the case of the function you point out, the patch series doesn't change the logic, rather it replaces the structure being used. >>>> Also, as you point out, a bootloader could read log contents from >>>> the shared region. Of course, in order for either to happen, >>>> the bootloader would already be compromised and I'm not sure that >>>> reading log entries or crashing the kernel is such a big consideration. >>> >>> The log might contain addresses of some kernel structures, for example >>> in some error messages. For this reason, only a privileged users are >>> able to read it. We must make sure that they will not get accessible >>> for a non-privileged user, for example via an address to the shared >>> buffer in the bootloader config. >> >> Ahhh, if I am understanding correctly, your concern is that an >> unprivileged program in user space would be able to gain access directly >> to the log entries via the external buffer location. I haven't tested >> that scenario, so I'm not sure if the memory reservation via the DT >> protects that memory from user space. I'll have to check on that. In >> the meantime, if you know of standard ways to protect memory from user >> space access, please let me know. > > I am not expert. I think that the main trick on the memory > management level is that the userspace and kernel have different > address spaces. IMHO, the pages that are used by the kernel log buffer > cannot be addressed from userspace. In fact, each user have only very > limited number of pages that are mapped to his address space. > > I do not know how this works with the persistent memory. > > In each case, we need to make sure that the address of the > shared buffer is not readable in the booloaded config file > for a normal user. I do not agree with that last point. If you cannot trust the bootloader then the ability of a userspace program to read log contents seems to be the least of your problems. >>> BTW: You did not answered the question about how the bootloader would >>
[RFC PATCH v2 2/2] printk: external log buffer (CONFIG_LOGBUFFER)
This debug feature provides a convenient way to collect log entries across
multiple, warmboot cycles and to share those entries with a boot loader.
It allows the kernel to use an external buffer for kernel log messages and
is controlled by an optional command line parameter. The buffer can contain
existing log messages from previous boot cycles and/or the bootloader. The
command line parameter was chosen for flexibility, cross arch portability,
and the ability to dynamically enable/disable this feature. The parameter
specifies the address of a control block used to replace the default log
buffer. Existing bootloader and kernel log messages are kept, in order,
inside the new buffer. After a boot that preserves the buffer contents, a
bootloader can display both kernel and bootloader log entries from
multiple, previous boots. It also allows the kernel to display bootloader
log entries along with its own messages.
This feature is intended for debug purposes and has no effect unless the
command line parameter is specified. Further, it validates the passed
control block carefully and if any checks fail, it falls back to the
default behaviour. As such, it can be left enabled by default.
Memory Reservation
--
This feature expects the bootloader to reserve/preserve the shared buffer
memory. This reservation needs to prevent the kernel from overwriting the
external log control block and log entries. In my testing, I've used the
'fdt' commands in uboot to dynamically inject reserved memory regions via
the DT to the kernel.
Based on the initial work of Wolfgang Denk and Igor Lisitsin [1].
Also based on work by: Alexander Streit
<alexander.str...@continental-corporation.com>
[1]
http://git.denx.de/?p=linux-2.6-denx.git;a=commitdiff;h=212f61c7fd3b952a81d1459dd32a86a32ddfd4ce
Signed-off-by: Sean Hudson <sean_hud...@mentor.com>
---
init/Kconfig | 12 +++
init/main.c| 7 ++
kernel/printk/printk.c | 263 -
3 files changed, 277 insertions(+), 5 deletions(-)
diff --git a/init/Kconfig b/init/Kconfig
index cac3f09..746183b 100644
--- a/init/Kconfig
+++ b/init/Kconfig
@@ -1493,6 +1493,18 @@ config PRINTK
very difficult to diagnose system problems, saying N here is
strongly discouraged.
+config LOGBUFFER
+ bool "External logbuffer" if PRINTK
+ default n
+ depends on PRINTK
+ help
+ This option enables support for an alternative, "external" printk log
+ buffer. If memory contents are preserved, e.g. after a warmboot, this
+ provides a known location for the boot loader to read and display
printk
+ entries from the kernel. If desired, the bootloader can write its own
+ log entries which the kernel will display with its own log entries.
+ Further, this capability can be used across multiple warmboot cycles.
+
config PRINTK_NMI
def_bool y
depends on PRINTK
diff --git a/init/main.c b/init/main.c
index a8a58e2..a5a54b3 100644
--- a/init/main.c
+++ b/init/main.c
@@ -94,6 +94,12 @@ extern void init_IRQ(void);
extern void fork_init(void);
extern void radix_tree_init(void);
+#ifdef CONFIG_LOGBUFFER
+extern void setup_ext_logbuff(void);
+#else
+void setup_ext_logbuff(void) {}
+#endif
+
/*
* Debug helper: via this flag we know that we are in 'early bootup code'
* where only the boot processor is running with IRQ disabled. This means
@@ -535,6 +541,7 @@ asmlinkage __visible void __init start_kernel(void)
sort_main_extable();
trap_init();
mm_init();
+ setup_ext_logbuff();
/*
* Set up the scheduler prior starting any interrupts (such as the
diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c
index 063f01d..5cf147e 100644
--- a/kernel/printk/printk.c
+++ b/kernel/printk/printk.c
@@ -331,17 +331,17 @@ enum log_flags {
};
struct printk_log {
- u64 ts_nsec;/* timestamp in nanoseconds */
+#ifdef CONFIG_LOGBUFFER
+ u32 log_magic; /* sanity check number */
+#endif
u16 len;/* length of entire record */
u16 text_len; /* length of text buffer */
u16 dict_len; /* length of dictionary buffer */
u8 facility;/* syslog facility */
u8 flags:5; /* internal record flags */
u8 level:3; /* syslog level */
+ u64 ts_nsec;/* timestamp in nanoseconds */
}
-#ifdef CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS
-__packed __aligned(4)
-#endif
;
/*
@@ -352,7 +352,14 @@ __packed __aligned(4)
DEFINE_RAW_SPINLOCK(logbuf_lock);
/*
- * This control block collects tracking offsets for the log.
+ * This control block collects tracking offsets for the log into a single
+ * place. It also facilitates pointing the log to another location, and,
+ * when combined with the CONFIG_LOGBUFFER feature, i
[RFC PATCH v2 1/2] printk: collect offsets into replaceable structure
Currently, printk relies on several indices that are declared as static
global variables in printk.c. This patch collects those into a single
structure referenced by a pointer. This allows easier replacement of
these indices and pinning to a specific locatino.
Signed-off-by: Sean Hudson <sean_hud...@mentor.com>
---
kernel/printk/printk.c | 338 +
1 file changed, 175 insertions(+), 163 deletions(-)
diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c
index eea6dbc..063f01d 100644
--- a/kernel/printk/printk.c
+++ b/kernel/printk/printk.c
@@ -351,30 +351,40 @@ __packed __aligned(4)
*/
DEFINE_RAW_SPINLOCK(logbuf_lock);
+/*
+ * This control block collects tracking offsets for the log.
+ */
+struct lcb_t {
+ /* Pointer to log buffer space and length of space */
+ char *log_buf;
+ u32 log_buf_len;
+
+ /* index and sequence of the first record stored in the buffer */
+ u64 log_first_seq;
+ u32 log_first_idx;
+
+ /* index and sequence of the next record to store in the buffer */
+ u64 log_next_seq;
+ u32 log_next_idx;
+
+ /* the next printk record to read by syslog(READ) or /proc/kmsg */
+ u64 syslog_seq;
+ u32 syslog_idx;
+ enum log_flags syslog_prev;
+ size_t syslog_partial;
+
+ /* the next printk record to write to the console */
+ u64 console_seq;
+ u32 console_idx;
+ enum log_flags console_prev;
+
+ /* the next printk record to read after the last 'clear' command */
+ u64 clear_seq;
+ u32 clear_idx;
+};
+
#ifdef CONFIG_PRINTK
DECLARE_WAIT_QUEUE_HEAD(log_wait);
-/* the next printk record to read by syslog(READ) or /proc/kmsg */
-static u64 syslog_seq;
-static u32 syslog_idx;
-static enum log_flags syslog_prev;
-static size_t syslog_partial;
-
-/* index and sequence number of the first record stored in the buffer */
-static u64 log_first_seq;
-static u32 log_first_idx;
-
-/* index and sequence number of the next record to store in the buffer */
-static u64 log_next_seq;
-static u32 log_next_idx;
-
-/* the next printk record to write to the console */
-static u64 console_seq;
-static u32 console_idx;
-static enum log_flags console_prev;
-
-/* the next printk record to read after the last 'clear' command */
-static u64 clear_seq;
-static u32 clear_idx;
#define PREFIX_MAX 32
#define LOG_LINE_MAX (1024 - PREFIX_MAX)
@@ -386,19 +396,22 @@ static u32 clear_idx;
#define LOG_ALIGN __alignof__(struct printk_log)
#define __LOG_BUF_LEN (1 << CONFIG_LOG_BUF_SHIFT)
static char __log_buf[__LOG_BUF_LEN] __aligned(LOG_ALIGN);
-static char *log_buf = __log_buf;
-static u32 log_buf_len = __LOG_BUF_LEN;
+
+static struct lcb_t __lcb = {
+ __log_buf, __LOG_BUF_LEN, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0 };
+
+static struct lcb_t *lcb = &__lcb;
/* Return log buffer address */
char *log_buf_addr_get(void)
{
- return log_buf;
+ return lcb->log_buf;
}
/* Return log buffer size */
u32 log_buf_len_get(void)
{
- return log_buf_len;
+ return lcb->log_buf_len;
}
/* human readable text of the record */
@@ -416,21 +429,21 @@ static char *log_dict(const struct printk_log *msg)
/* get record by index; idx must point to valid msg */
static struct printk_log *log_from_idx(u32 idx)
{
- struct printk_log *msg = (struct printk_log *)(log_buf + idx);
+ struct printk_log *msg = (struct printk_log *)(lcb->log_buf + idx);
/*
* A length == 0 record is the end of buffer marker. Wrap around and
* read the message at the start of the buffer.
*/
if (!msg->len)
- return (struct printk_log *)log_buf;
+ return (struct printk_log *)lcb->log_buf;
return msg;
}
/* get next record; idx must point to valid msg */
static u32 log_next(u32 idx)
{
- struct printk_log *msg = (struct printk_log *)(log_buf + idx);
+ struct printk_log *msg = (struct printk_log *)(lcb->log_buf + idx);
/* length == 0 indicates the end of the buffer; wrap */
/*
@@ -439,7 +452,7 @@ static u32 log_next(u32 idx)
* return the one after that.
*/
if (!msg->len) {
- msg = (struct printk_log *)log_buf;
+ msg = (struct printk_log *)lcb->log_buf;
return msg->len;
}
return idx + msg->len;
@@ -458,10 +471,11 @@ static int logbuf_has_space(u32 msg_size, bool empty)
{
u32 free;
- if (log_next_idx > log_first_idx || empty)
- free = max(log_buf_len - log_next_idx, log_first_idx);
+ if (lcb->log_next_idx > lcb->log_first_idx || empty)
+ free = max(lcb->log_buf_len - lcb->log_next_idx,
+ lcb->log_first_idx);
else
- free = log_first_idx - log_next
[RFC PATCH v2 2/2] printk: external log buffer (CONFIG_LOGBUFFER)
This debug feature provides a convenient way to collect log entries across
multiple, warmboot cycles and to share those entries with a boot loader.
It allows the kernel to use an external buffer for kernel log messages and
is controlled by an optional command line parameter. The buffer can contain
existing log messages from previous boot cycles and/or the bootloader. The
command line parameter was chosen for flexibility, cross arch portability,
and the ability to dynamically enable/disable this feature. The parameter
specifies the address of a control block used to replace the default log
buffer. Existing bootloader and kernel log messages are kept, in order,
inside the new buffer. After a boot that preserves the buffer contents, a
bootloader can display both kernel and bootloader log entries from
multiple, previous boots. It also allows the kernel to display bootloader
log entries along with its own messages.
This feature is intended for debug purposes and has no effect unless the
command line parameter is specified. Further, it validates the passed
control block carefully and if any checks fail, it falls back to the
default behaviour. As such, it can be left enabled by default.
Memory Reservation
--
This feature expects the bootloader to reserve/preserve the shared buffer
memory. This reservation needs to prevent the kernel from overwriting the
external log control block and log entries. In my testing, I've used the
'fdt' commands in uboot to dynamically inject reserved memory regions via
the DT to the kernel.
Based on the initial work of Wolfgang Denk and Igor Lisitsin [1].
Also based on work by: Alexander Streit
[1]
http://git.denx.de/?p=linux-2.6-denx.git;a=commitdiff;h=212f61c7fd3b952a81d1459dd32a86a32ddfd4ce
Signed-off-by: Sean Hudson
---
init/Kconfig | 12 +++
init/main.c| 7 ++
kernel/printk/printk.c | 263 -
3 files changed, 277 insertions(+), 5 deletions(-)
diff --git a/init/Kconfig b/init/Kconfig
index cac3f09..746183b 100644
--- a/init/Kconfig
+++ b/init/Kconfig
@@ -1493,6 +1493,18 @@ config PRINTK
very difficult to diagnose system problems, saying N here is
strongly discouraged.
+config LOGBUFFER
+ bool "External logbuffer" if PRINTK
+ default n
+ depends on PRINTK
+ help
+ This option enables support for an alternative, "external" printk log
+ buffer. If memory contents are preserved, e.g. after a warmboot, this
+ provides a known location for the boot loader to read and display
printk
+ entries from the kernel. If desired, the bootloader can write its own
+ log entries which the kernel will display with its own log entries.
+ Further, this capability can be used across multiple warmboot cycles.
+
config PRINTK_NMI
def_bool y
depends on PRINTK
diff --git a/init/main.c b/init/main.c
index a8a58e2..a5a54b3 100644
--- a/init/main.c
+++ b/init/main.c
@@ -94,6 +94,12 @@ extern void init_IRQ(void);
extern void fork_init(void);
extern void radix_tree_init(void);
+#ifdef CONFIG_LOGBUFFER
+extern void setup_ext_logbuff(void);
+#else
+void setup_ext_logbuff(void) {}
+#endif
+
/*
* Debug helper: via this flag we know that we are in 'early bootup code'
* where only the boot processor is running with IRQ disabled. This means
@@ -535,6 +541,7 @@ asmlinkage __visible void __init start_kernel(void)
sort_main_extable();
trap_init();
mm_init();
+ setup_ext_logbuff();
/*
* Set up the scheduler prior starting any interrupts (such as the
diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c
index 063f01d..5cf147e 100644
--- a/kernel/printk/printk.c
+++ b/kernel/printk/printk.c
@@ -331,17 +331,17 @@ enum log_flags {
};
struct printk_log {
- u64 ts_nsec;/* timestamp in nanoseconds */
+#ifdef CONFIG_LOGBUFFER
+ u32 log_magic; /* sanity check number */
+#endif
u16 len;/* length of entire record */
u16 text_len; /* length of text buffer */
u16 dict_len; /* length of dictionary buffer */
u8 facility;/* syslog facility */
u8 flags:5; /* internal record flags */
u8 level:3; /* syslog level */
+ u64 ts_nsec;/* timestamp in nanoseconds */
}
-#ifdef CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS
-__packed __aligned(4)
-#endif
;
/*
@@ -352,7 +352,14 @@ __packed __aligned(4)
DEFINE_RAW_SPINLOCK(logbuf_lock);
/*
- * This control block collects tracking offsets for the log.
+ * This control block collects tracking offsets for the log into a single
+ * place. It also facilitates pointing the log to another location, and,
+ * when combined with the CONFIG_LOGBUFFER feature, it allows log sharing
+ * between the bootloader and the kernel.
+ *
+ * NOTE:
[RFC PATCH v2 1/2] printk: collect offsets into replaceable structure
Currently, printk relies on several indices that are declared as static
global variables in printk.c. This patch collects those into a single
structure referenced by a pointer. This allows easier replacement of
these indices and pinning to a specific locatino.
Signed-off-by: Sean Hudson
---
kernel/printk/printk.c | 338 +
1 file changed, 175 insertions(+), 163 deletions(-)
diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c
index eea6dbc..063f01d 100644
--- a/kernel/printk/printk.c
+++ b/kernel/printk/printk.c
@@ -351,30 +351,40 @@ __packed __aligned(4)
*/
DEFINE_RAW_SPINLOCK(logbuf_lock);
+/*
+ * This control block collects tracking offsets for the log.
+ */
+struct lcb_t {
+ /* Pointer to log buffer space and length of space */
+ char *log_buf;
+ u32 log_buf_len;
+
+ /* index and sequence of the first record stored in the buffer */
+ u64 log_first_seq;
+ u32 log_first_idx;
+
+ /* index and sequence of the next record to store in the buffer */
+ u64 log_next_seq;
+ u32 log_next_idx;
+
+ /* the next printk record to read by syslog(READ) or /proc/kmsg */
+ u64 syslog_seq;
+ u32 syslog_idx;
+ enum log_flags syslog_prev;
+ size_t syslog_partial;
+
+ /* the next printk record to write to the console */
+ u64 console_seq;
+ u32 console_idx;
+ enum log_flags console_prev;
+
+ /* the next printk record to read after the last 'clear' command */
+ u64 clear_seq;
+ u32 clear_idx;
+};
+
#ifdef CONFIG_PRINTK
DECLARE_WAIT_QUEUE_HEAD(log_wait);
-/* the next printk record to read by syslog(READ) or /proc/kmsg */
-static u64 syslog_seq;
-static u32 syslog_idx;
-static enum log_flags syslog_prev;
-static size_t syslog_partial;
-
-/* index and sequence number of the first record stored in the buffer */
-static u64 log_first_seq;
-static u32 log_first_idx;
-
-/* index and sequence number of the next record to store in the buffer */
-static u64 log_next_seq;
-static u32 log_next_idx;
-
-/* the next printk record to write to the console */
-static u64 console_seq;
-static u32 console_idx;
-static enum log_flags console_prev;
-
-/* the next printk record to read after the last 'clear' command */
-static u64 clear_seq;
-static u32 clear_idx;
#define PREFIX_MAX 32
#define LOG_LINE_MAX (1024 - PREFIX_MAX)
@@ -386,19 +396,22 @@ static u32 clear_idx;
#define LOG_ALIGN __alignof__(struct printk_log)
#define __LOG_BUF_LEN (1 << CONFIG_LOG_BUF_SHIFT)
static char __log_buf[__LOG_BUF_LEN] __aligned(LOG_ALIGN);
-static char *log_buf = __log_buf;
-static u32 log_buf_len = __LOG_BUF_LEN;
+
+static struct lcb_t __lcb = {
+ __log_buf, __LOG_BUF_LEN, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0 };
+
+static struct lcb_t *lcb = &__lcb;
/* Return log buffer address */
char *log_buf_addr_get(void)
{
- return log_buf;
+ return lcb->log_buf;
}
/* Return log buffer size */
u32 log_buf_len_get(void)
{
- return log_buf_len;
+ return lcb->log_buf_len;
}
/* human readable text of the record */
@@ -416,21 +429,21 @@ static char *log_dict(const struct printk_log *msg)
/* get record by index; idx must point to valid msg */
static struct printk_log *log_from_idx(u32 idx)
{
- struct printk_log *msg = (struct printk_log *)(log_buf + idx);
+ struct printk_log *msg = (struct printk_log *)(lcb->log_buf + idx);
/*
* A length == 0 record is the end of buffer marker. Wrap around and
* read the message at the start of the buffer.
*/
if (!msg->len)
- return (struct printk_log *)log_buf;
+ return (struct printk_log *)lcb->log_buf;
return msg;
}
/* get next record; idx must point to valid msg */
static u32 log_next(u32 idx)
{
- struct printk_log *msg = (struct printk_log *)(log_buf + idx);
+ struct printk_log *msg = (struct printk_log *)(lcb->log_buf + idx);
/* length == 0 indicates the end of the buffer; wrap */
/*
@@ -439,7 +452,7 @@ static u32 log_next(u32 idx)
* return the one after that.
*/
if (!msg->len) {
- msg = (struct printk_log *)log_buf;
+ msg = (struct printk_log *)lcb->log_buf;
return msg->len;
}
return idx + msg->len;
@@ -458,10 +471,11 @@ static int logbuf_has_space(u32 msg_size, bool empty)
{
u32 free;
- if (log_next_idx > log_first_idx || empty)
- free = max(log_buf_len - log_next_idx, log_first_idx);
+ if (lcb->log_next_idx > lcb->log_first_idx || empty)
+ free = max(lcb->log_buf_len - lcb->log_next_idx,
+ lcb->log_first_idx);
else
- free = log_first_idx - log_next_idx;
+ free = lcb->log_first_idx
[RFC PATCH v2 0/2] printk: Shared kernel logging
This patch set is based on Linus' v4.8-rc8 tag. This debug feature allows the kernel to use an external buffer and control block for kernel log messages. The feature is controlled by an optional command line parameter. The existing buffer and control block can contain existing log messages from previous boot cycles and/or the bootloader. The command line parameter was chosen for flexibility, cross arch portability, and the ability to dynamically enable/disable this feature. The parameter specifies the address of a control block used to replace the default log buffer. Existing bootloader and kernel log messages are kept, in order, inside the new buffer. After a boot that preserves the buffer contents, a bootloader can display both kernel and bootloader log entries from multiple, previous boots. It also allows the kernel to display bootloader log entries along with its own messages. This feature is intended for debug purposes and has no effect unless the command line parameter is specified. Further, it validates the passed control block carefully and if any checks fail, it falls back to the default behaviour. As such, it can be left enabled by default. Memory Reservation This feature expects the bootloader to reserve/preserve the shared buffer memory. This reservation needs to prevent the kernel from overwriting the external log control block and log entries. In my testing, I've used the 'fdt' commands in uboot to dynamically inject reserved memory regions via the DT to the kernel. Sean Hudson (2): printk: collect offsets into replaceable structure printk: external log buffer (CONFIG_LOGBUFFER) init/Kconfig | 12 + init/main.c| 7 + kernel/printk/printk.c | 599 +++-- 3 files changed, 451 insertions(+), 167 deletions(-) Changes since v1: Fix CONFIG_PRINTK disabled build -- 2.7.4
[RFC PATCH v2 0/2] printk: Shared kernel logging
This patch set is based on Linus' v4.8-rc8 tag. This debug feature allows the kernel to use an external buffer and control block for kernel log messages. The feature is controlled by an optional command line parameter. The existing buffer and control block can contain existing log messages from previous boot cycles and/or the bootloader. The command line parameter was chosen for flexibility, cross arch portability, and the ability to dynamically enable/disable this feature. The parameter specifies the address of a control block used to replace the default log buffer. Existing bootloader and kernel log messages are kept, in order, inside the new buffer. After a boot that preserves the buffer contents, a bootloader can display both kernel and bootloader log entries from multiple, previous boots. It also allows the kernel to display bootloader log entries along with its own messages. This feature is intended for debug purposes and has no effect unless the command line parameter is specified. Further, it validates the passed control block carefully and if any checks fail, it falls back to the default behaviour. As such, it can be left enabled by default. Memory Reservation This feature expects the bootloader to reserve/preserve the shared buffer memory. This reservation needs to prevent the kernel from overwriting the external log control block and log entries. In my testing, I've used the 'fdt' commands in uboot to dynamically inject reserved memory regions via the DT to the kernel. Sean Hudson (2): printk: collect offsets into replaceable structure printk: external log buffer (CONFIG_LOGBUFFER) init/Kconfig | 12 + init/main.c| 7 + kernel/printk/printk.c | 599 +++-- 3 files changed, 451 insertions(+), 167 deletions(-) Changes since v1: Fix CONFIG_PRINTK disabled build -- 2.7.4
Re: [RFC PATCH v1 2/2] printk: external log buffer (CONFIG_LOGBUFFER)
On 10/4/2016 6:27 AM, Petr Mladek wrote: > On Fri 2016-09-30 23:06:49, Sean Hudson wrote: >> On 9/30/2016 7:39 AM, Petr Mladek wrote: >>> >>> Is the bootloader allowed to write to the shared buffer from this >>> point on? How will be synchronized the writes from the kernel and >>> the bootloader? >> >> I am a bit confused by this question. I assume that the kernel and >> bootloader will not be accessing the same log space simultaneously. >> While each one is running, they will utilize the same data structures to >> keep track of current entries. > > They both will write new messages at the end of the buffer. Therefore > they both might want to write to exactly the same location. Also if > the buffer is full, the oldest messages are overwritten. Therefore > even reading must be synchronized against writes. > > These operations are synchronized using logbuf_lock on the kernel side. > I do not know much about bootloaders but I guess that the bootloader > will not be able to use the logbuf_lock. So the question is how > the bootloader and kernel will synchronize the access to the shared > buffer. > > Or did I miss anything? The state of the log is preserved in the log control block, or lcb, which is passed when the boot loader passes control to the kernel. In order to make the transition as seamless as possible, the boot loader appends entries in the same way that the kernel does and overwrites the oldest entries once the ring buffer is full. It also updates tracking indices in the lcb. When the boot loader passes control to the kernel, the kernel adopts the lcb and appends entries using that structure. In this way, the lcb provides continuity between the boot loader and the kernel for the state of the buffer. Note, there is a small window of time during early boot in which the kernel puts entries into the default location, a staticlly declared char[]. These entries are appended to the external location once setup_ext_logbuff() is called from init/main.c. (I'd love to close that gap entirely, but all solutions I've tried involve arch specific code, which I'm trying to avoid.) >>> How protected will the address of the shared buffer? Would not >>> it open a security hole? We need to make sure that non-privileged >>> user or non-trusted application must not read sensitive data >>> from the log or break the structure of the log by writing >>> invalid data. >> >> There is no notion of protection of the address. However, additional >> checking could be added. Currently, there is potential for corrupt log >> entries from the bootloader causing the kernel to crash. >> I can't think of a way to prevent that from happening. > > IMHO, the only way is to revisit all locations when the log buffer > is accessed and add all the needed consistency checks. We must make > sure that we do not access outside of the buffer. Also the log_*_seq > numbers are supposed to grow linearly... Adding an explicit walk of the external log entries should be trivial. That will only validate that the passed in log entries are consistent internally. I will be submitting a v2 patch shortly that fixes an error that kernel-ci caught. (if CONFIG_PRINTK is disabled, compilation fails) I'll see about adding that check at the same time. >> Also, as you point out, a bootloader could read log contents from >> the shared region. Of course, in order for either to happen, >> the bootloader would already be compromised and I'm not sure that >> reading log entries or crashing the kernel is such a big consideration. > > The log might contain addresses of some kernel structures, for example > in some error messages. For this reason, only a privileged users are > able to read it. We must make sure that they will not get accessible > for a non-privileged user, for example via an address to the shared > buffer in the bootloader config. Ahhh, if I am understanding correctly, your concern is that an unprivileged program in user space would be able to gain access directly to the log entries via the external buffer location. I haven't tested that scenario, so I'm not sure if the memory reservation via the DT protects that memory from user space. I'll have to check on that. In the meantime, if you know of standard ways to protect memory from user space access, please let me know. > BTW: You did not answered the question about how the bootloader would > know the right version of the log format. I am afraid that we do not > want to maintain backward compatibility on the kernel side. The printk > code already is too complex. This is a matter of magic numbers. Uboot has two preexisting log structure versions. Since the kernel did not have a version, I used LOGBUFF_LOG_VERSION = 3, if CO
Re: [RFC PATCH v1 2/2] printk: external log buffer (CONFIG_LOGBUFFER)
On 10/4/2016 6:27 AM, Petr Mladek wrote: > On Fri 2016-09-30 23:06:49, Sean Hudson wrote: >> On 9/30/2016 7:39 AM, Petr Mladek wrote: >>> >>> Is the bootloader allowed to write to the shared buffer from this >>> point on? How will be synchronized the writes from the kernel and >>> the bootloader? >> >> I am a bit confused by this question. I assume that the kernel and >> bootloader will not be accessing the same log space simultaneously. >> While each one is running, they will utilize the same data structures to >> keep track of current entries. > > They both will write new messages at the end of the buffer. Therefore > they both might want to write to exactly the same location. Also if > the buffer is full, the oldest messages are overwritten. Therefore > even reading must be synchronized against writes. > > These operations are synchronized using logbuf_lock on the kernel side. > I do not know much about bootloaders but I guess that the bootloader > will not be able to use the logbuf_lock. So the question is how > the bootloader and kernel will synchronize the access to the shared > buffer. > > Or did I miss anything? The state of the log is preserved in the log control block, or lcb, which is passed when the boot loader passes control to the kernel. In order to make the transition as seamless as possible, the boot loader appends entries in the same way that the kernel does and overwrites the oldest entries once the ring buffer is full. It also updates tracking indices in the lcb. When the boot loader passes control to the kernel, the kernel adopts the lcb and appends entries using that structure. In this way, the lcb provides continuity between the boot loader and the kernel for the state of the buffer. Note, there is a small window of time during early boot in which the kernel puts entries into the default location, a staticlly declared char[]. These entries are appended to the external location once setup_ext_logbuff() is called from init/main.c. (I'd love to close that gap entirely, but all solutions I've tried involve arch specific code, which I'm trying to avoid.) >>> How protected will the address of the shared buffer? Would not >>> it open a security hole? We need to make sure that non-privileged >>> user or non-trusted application must not read sensitive data >>> from the log or break the structure of the log by writing >>> invalid data. >> >> There is no notion of protection of the address. However, additional >> checking could be added. Currently, there is potential for corrupt log >> entries from the bootloader causing the kernel to crash. >> I can't think of a way to prevent that from happening. > > IMHO, the only way is to revisit all locations when the log buffer > is accessed and add all the needed consistency checks. We must make > sure that we do not access outside of the buffer. Also the log_*_seq > numbers are supposed to grow linearly... Adding an explicit walk of the external log entries should be trivial. That will only validate that the passed in log entries are consistent internally. I will be submitting a v2 patch shortly that fixes an error that kernel-ci caught. (if CONFIG_PRINTK is disabled, compilation fails) I'll see about adding that check at the same time. >> Also, as you point out, a bootloader could read log contents from >> the shared region. Of course, in order for either to happen, >> the bootloader would already be compromised and I'm not sure that >> reading log entries or crashing the kernel is such a big consideration. > > The log might contain addresses of some kernel structures, for example > in some error messages. For this reason, only a privileged users are > able to read it. We must make sure that they will not get accessible > for a non-privileged user, for example via an address to the shared > buffer in the bootloader config. Ahhh, if I am understanding correctly, your concern is that an unprivileged program in user space would be able to gain access directly to the log entries via the external buffer location. I haven't tested that scenario, so I'm not sure if the memory reservation via the DT protects that memory from user space. I'll have to check on that. In the meantime, if you know of standard ways to protect memory from user space access, please let me know. > BTW: You did not answered the question about how the bootloader would > know the right version of the log format. I am afraid that we do not > want to maintain backward compatibility on the kernel side. The printk > code already is too complex. This is a matter of magic numbers. Uboot has two preexisting log structure versions. Since the kernel did not have a version, I used LOGBUFF_LOG_VERSION = 3, if CO
Re: [RFC PATCH v1 2/2] printk: external log buffer (CONFIG_LOGBUFFER)
On 9/30/2016 7:39 AM, Petr Mladek wrote:
> On Thu 2016-09-29 19:55:56, Sean Hudson wrote:
>> This debug feature provides a convenient way to collect log entries across
>> multiple, warmboot cycles and to share those entries with a boot loader.
>> It allows the kernel to use an external buffer for kernel log messages and
>> is controlled by an optional command line parameter. The buffer can contain
>> existing log messages from previous boot cycles and/or the bootloader. The
>> command line parameter was chosen for flexibility, cross arch portability,
>> and the ability to dynamically enable/disable this feature. The parameter
>> specifies the address of a control block used to replace the default log
>> buffer. Existing bootloader and kernel log messages are kept, in order,
>> inside the new buffer. After a boot that preserves the buffer contents, a
>> bootloader can display both kernel and bootloader log entries from
>> multiple, previous boots. It also allows the kernel to display bootloader
>> log entries along with its own messages.
>>
>> This feature is intended for debug purposes and has no effect unless the
>> command line parameter is specified. Further, it validates the passed
>> control block carefully and if any checks fail, it falls back to the
>> default behaviour. As such, it can be left enabled by default.
>>
>> Memory Reservation
>> --
>> This feature expects the bootloader to reserve/preserve the shared buffer
>> memory. This reservation needs to prevent the kernel from overwriting the
>> external log control block and log entries. In my testing, I've used the
>> 'fdt' commands in uboot to dynamically inject reserved memory regions via
>> the DT to the kernel.
>>
>> Based on the initial work of Wolfgang Denk and Igor Lisitsin [1].
>> Also based on work by: Alexander Streit
>> <alexander.str...@continental-corporation.com>
>>
>> [1]
>> http://git.denx.de/?p=linux-2.6-denx.git;a=commitdiff;h=212f61c7fd3b952a81d1459dd32a86a32ddfd4ce
>>
>> Signed-off-by: Sean Hudson <sean_hud...@mentor.com>
>> ---
>> init/Kconfig | 12 +++
>> init/main.c| 2 +
>> kernel/printk/printk.c | 267
>> +++--
>> 3 files changed, 275 insertions(+), 6 deletions(-)
>>
>> diff --git a/init/Kconfig b/init/Kconfig
>> index cac3f09..746183b 100644
>> --- a/init/Kconfig
>> +++ b/init/Kconfig
>> @@ -1493,6 +1493,18 @@ config PRINTK
>>very difficult to diagnose system problems, saying N here is
>>strongly discouraged.
>>
>> +config LOGBUFFER
>> +bool "External logbuffer" if PRINTK
>> +default n
>> +depends on PRINTK
>> +help
>> + This option enables support for an alternative, "external" printk log
>> + buffer. If memory contents are preserved, e.g. after a warmboot, this
>> + provides a known location for the boot loader to read and display
>> printk
>> + entries from the kernel. If desired, the bootloader can write its own
>> + log entries which the kernel will display with its own log entries.
>> + Further, this capability can be used across multiple warmboot cycles.
>> +
>> config PRINTK_NMI
>> def_bool y
>> depends on PRINTK
>> diff --git a/init/main.c b/init/main.c
>> index a8a58e2..4a5913c 100644
>> --- a/init/main.c
>> +++ b/init/main.c
>> @@ -93,6 +93,7 @@ static int kernel_init(void *);
>> extern void init_IRQ(void);
>> extern void fork_init(void);
>> extern void radix_tree_init(void);
>> +extern void setup_ext_logbuff(void);
>>
>> /*
>> * Debug helper: via this flag we know that we are in 'early bootup code'
>> @@ -535,6 +536,7 @@ asmlinkage __visible void __init start_kernel(void)
>> sort_main_extable();
>> trap_init();
>> mm_init();
>> +setup_ext_logbuff();
>>
>> /*
>> * Set up the scheduler prior starting any interrupts (such as the
>> diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c
>> index 7a441f5..017b4d4 100644
>> --- a/kernel/printk/printk.c
>> +++ b/kernel/printk/printk.c
>> @@ -395,13 +402,42 @@ struct lcb_t {
>> +/*
>> + * The bootloader may write an unexpected version/format of the log, so
>> + * these values provide a way to check that the format agrees.
>> + */
>> +#define LOGBUFF_MAGIC 0xc0de4ced
>> +#define LOGBUFF_LOG_VE
Re: [RFC PATCH v1 2/2] printk: external log buffer (CONFIG_LOGBUFFER)
On 9/30/2016 7:39 AM, Petr Mladek wrote:
> On Thu 2016-09-29 19:55:56, Sean Hudson wrote:
>> This debug feature provides a convenient way to collect log entries across
>> multiple, warmboot cycles and to share those entries with a boot loader.
>> It allows the kernel to use an external buffer for kernel log messages and
>> is controlled by an optional command line parameter. The buffer can contain
>> existing log messages from previous boot cycles and/or the bootloader. The
>> command line parameter was chosen for flexibility, cross arch portability,
>> and the ability to dynamically enable/disable this feature. The parameter
>> specifies the address of a control block used to replace the default log
>> buffer. Existing bootloader and kernel log messages are kept, in order,
>> inside the new buffer. After a boot that preserves the buffer contents, a
>> bootloader can display both kernel and bootloader log entries from
>> multiple, previous boots. It also allows the kernel to display bootloader
>> log entries along with its own messages.
>>
>> This feature is intended for debug purposes and has no effect unless the
>> command line parameter is specified. Further, it validates the passed
>> control block carefully and if any checks fail, it falls back to the
>> default behaviour. As such, it can be left enabled by default.
>>
>> Memory Reservation
>> --
>> This feature expects the bootloader to reserve/preserve the shared buffer
>> memory. This reservation needs to prevent the kernel from overwriting the
>> external log control block and log entries. In my testing, I've used the
>> 'fdt' commands in uboot to dynamically inject reserved memory regions via
>> the DT to the kernel.
>>
>> Based on the initial work of Wolfgang Denk and Igor Lisitsin [1].
>> Also based on work by: Alexander Streit
>>
>>
>> [1]
>> http://git.denx.de/?p=linux-2.6-denx.git;a=commitdiff;h=212f61c7fd3b952a81d1459dd32a86a32ddfd4ce
>>
>> Signed-off-by: Sean Hudson
>> ---
>> init/Kconfig | 12 +++
>> init/main.c| 2 +
>> kernel/printk/printk.c | 267
>> +++--
>> 3 files changed, 275 insertions(+), 6 deletions(-)
>>
>> diff --git a/init/Kconfig b/init/Kconfig
>> index cac3f09..746183b 100644
>> --- a/init/Kconfig
>> +++ b/init/Kconfig
>> @@ -1493,6 +1493,18 @@ config PRINTK
>>very difficult to diagnose system problems, saying N here is
>>strongly discouraged.
>>
>> +config LOGBUFFER
>> +bool "External logbuffer" if PRINTK
>> +default n
>> +depends on PRINTK
>> +help
>> + This option enables support for an alternative, "external" printk log
>> + buffer. If memory contents are preserved, e.g. after a warmboot, this
>> + provides a known location for the boot loader to read and display
>> printk
>> + entries from the kernel. If desired, the bootloader can write its own
>> + log entries which the kernel will display with its own log entries.
>> + Further, this capability can be used across multiple warmboot cycles.
>> +
>> config PRINTK_NMI
>> def_bool y
>> depends on PRINTK
>> diff --git a/init/main.c b/init/main.c
>> index a8a58e2..4a5913c 100644
>> --- a/init/main.c
>> +++ b/init/main.c
>> @@ -93,6 +93,7 @@ static int kernel_init(void *);
>> extern void init_IRQ(void);
>> extern void fork_init(void);
>> extern void radix_tree_init(void);
>> +extern void setup_ext_logbuff(void);
>>
>> /*
>> * Debug helper: via this flag we know that we are in 'early bootup code'
>> @@ -535,6 +536,7 @@ asmlinkage __visible void __init start_kernel(void)
>> sort_main_extable();
>> trap_init();
>> mm_init();
>> +setup_ext_logbuff();
>>
>> /*
>> * Set up the scheduler prior starting any interrupts (such as the
>> diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c
>> index 7a441f5..017b4d4 100644
>> --- a/kernel/printk/printk.c
>> +++ b/kernel/printk/printk.c
>> @@ -395,13 +402,42 @@ struct lcb_t {
>> +/*
>> + * The bootloader may write an unexpected version/format of the log, so
>> + * these values provide a way to check that the format agrees.
>> + */
>> +#define LOGBUFF_MAGIC 0xc0de4ced
>> +#define LOGBUFF_LOG_VERSION 3
>> +#define LOGBUFF_CB_PADDED_LENGTH1024
>>
>> static struct lcb_t __lcb
Re: [RFC PATCH v1 0/2] printk: Shared kernel logging
On 9/29/2016 8:36 PM, Kees Cook wrote: > On Thu, Sep 29, 2016 at 5:55 PM, Sean Hudson <sean_hud...@mentor.com> > wrote: >> This patch set is based on Linus' v4.8-rc8 tag. >> >> This debug feature allows the kernel to use an external buffer and >> control block for kernel log messages. The feature is controlled by >> an optional command line parameter. The existing buffer and control >> block can contain existing log messages from previous boot cycles >> and/or the bootloader. The command line parameter was chosen for >> flexibility, cross arch portability, and the ability to dynamically >> enable/disable this feature. The parameter specifies the address of >> a control block used to replace the default log buffer. Existing >> bootloader and kernel log messages are kept, in order, inside the >> new buffer. After a boot that preserves the buffer contents, a >> bootloader can display both kernel and bootloader log entries from >> multiple, previous boots. It also allows the kernel to display >> bootloader log entries along with its own messages. >> >> This feature is intended for debug purposes and has no effect >> unless the command line parameter is specified. Further, it >> validates the passed control block carefully and if any checks >> fail, it falls back to the default behaviour. As such, it can be >> left enabled by default. >> >> Memory Reservation >> >> This feature expects the bootloader to reserve/preserve the shared >> buffer memory. This reservation needs to prevent the kernel from >> overwriting the external log control block and log entries. In my >> testing, I've used the 'fdt' commands in uboot to dynamically >> inject reserved memory regions via the DT to the kernel. > > Interesting! I wonder if this can be adjusted to incorporate the > existing console logging feature in the pstore which does a similar > thing? Though pstore doesn't know about bootloader logs, really, > it's just storing kernel logs in a ring buffer. Maybe this can > provide a backend to pstore or something, especially since pstore > initialization happens "too late" for this to really be very > sensible. It just seems like it'd be nice to have a single persistent > console memory region... > I don't know that much about pstore. From your description though, it sounds feasible to put the two together at some point. How arch specific is pstore? -- Sean signature.asc Description: OpenPGP digital signature
Re: [RFC PATCH v1 0/2] printk: Shared kernel logging
On 9/29/2016 8:36 PM, Kees Cook wrote: > On Thu, Sep 29, 2016 at 5:55 PM, Sean Hudson > wrote: >> This patch set is based on Linus' v4.8-rc8 tag. >> >> This debug feature allows the kernel to use an external buffer and >> control block for kernel log messages. The feature is controlled by >> an optional command line parameter. The existing buffer and control >> block can contain existing log messages from previous boot cycles >> and/or the bootloader. The command line parameter was chosen for >> flexibility, cross arch portability, and the ability to dynamically >> enable/disable this feature. The parameter specifies the address of >> a control block used to replace the default log buffer. Existing >> bootloader and kernel log messages are kept, in order, inside the >> new buffer. After a boot that preserves the buffer contents, a >> bootloader can display both kernel and bootloader log entries from >> multiple, previous boots. It also allows the kernel to display >> bootloader log entries along with its own messages. >> >> This feature is intended for debug purposes and has no effect >> unless the command line parameter is specified. Further, it >> validates the passed control block carefully and if any checks >> fail, it falls back to the default behaviour. As such, it can be >> left enabled by default. >> >> Memory Reservation >> >> This feature expects the bootloader to reserve/preserve the shared >> buffer memory. This reservation needs to prevent the kernel from >> overwriting the external log control block and log entries. In my >> testing, I've used the 'fdt' commands in uboot to dynamically >> inject reserved memory regions via the DT to the kernel. > > Interesting! I wonder if this can be adjusted to incorporate the > existing console logging feature in the pstore which does a similar > thing? Though pstore doesn't know about bootloader logs, really, > it's just storing kernel logs in a ring buffer. Maybe this can > provide a backend to pstore or something, especially since pstore > initialization happens "too late" for this to really be very > sensible. It just seems like it'd be nice to have a single persistent > console memory region... > I don't know that much about pstore. From your description though, it sounds feasible to put the two together at some point. How arch specific is pstore? -- Sean signature.asc Description: OpenPGP digital signature
[RFC PATCH v1 1/2] printk: collect offsets into replaceable structure
Currently, printk relies on several indices that are declared as static
global variables in printk.c. This patch collects those into a single
structure referenced by a pointer. This allows easier replacement of
these indices and pinning to a specific locatino.
Signed-off-by: Sean Hudson <sean_hud...@mentor.com>
---
kernel/printk/printk.c | 335 +
1 file changed, 172 insertions(+), 163 deletions(-)
diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c
index eea6dbc..7a441f5 100644
--- a/kernel/printk/printk.c
+++ b/kernel/printk/printk.c
@@ -353,28 +353,6 @@ DEFINE_RAW_SPINLOCK(logbuf_lock);
#ifdef CONFIG_PRINTK
DECLARE_WAIT_QUEUE_HEAD(log_wait);
-/* the next printk record to read by syslog(READ) or /proc/kmsg */
-static u64 syslog_seq;
-static u32 syslog_idx;
-static enum log_flags syslog_prev;
-static size_t syslog_partial;
-
-/* index and sequence number of the first record stored in the buffer */
-static u64 log_first_seq;
-static u32 log_first_idx;
-
-/* index and sequence number of the next record to store in the buffer */
-static u64 log_next_seq;
-static u32 log_next_idx;
-
-/* the next printk record to write to the console */
-static u64 console_seq;
-static u32 console_idx;
-static enum log_flags console_prev;
-
-/* the next printk record to read after the last 'clear' command */
-static u64 clear_seq;
-static u32 clear_idx;
#define PREFIX_MAX 32
#define LOG_LINE_MAX (1024 - PREFIX_MAX)
@@ -386,19 +364,54 @@ static u32 clear_idx;
#define LOG_ALIGN __alignof__(struct printk_log)
#define __LOG_BUF_LEN (1 << CONFIG_LOG_BUF_SHIFT)
static char __log_buf[__LOG_BUF_LEN] __aligned(LOG_ALIGN);
-static char *log_buf = __log_buf;
-static u32 log_buf_len = __LOG_BUF_LEN;
+
+/*
+ * This control block collects tracking offsets for the log.
+ */
+struct lcb_t {
+ /* Pointer to log buffer space and length of space */
+ char *log_buf;
+ u32 log_buf_len;
+
+ /* index and sequence of the first record stored in the buffer */
+ u64 log_first_seq;
+ u32 log_first_idx;
+
+ /* index and sequence of the next record to store in the buffer */
+ u64 log_next_seq;
+ u32 log_next_idx;
+
+ /* the next printk record to read by syslog(READ) or /proc/kmsg */
+ u64 syslog_seq;
+ u32 syslog_idx;
+ enum log_flags syslog_prev;
+ size_t syslog_partial;
+
+ /* the next printk record to write to the console */
+ u64 console_seq;
+ u32 console_idx;
+ enum log_flags console_prev;
+
+ /* the next printk record to read after the last 'clear' command */
+ u64 clear_seq;
+ u32 clear_idx;
+} lcb_t;
+
+static struct lcb_t __lcb = {
+ __log_buf, __LOG_BUF_LEN, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0 };
+
+static struct lcb_t *lcb = &__lcb;
/* Return log buffer address */
char *log_buf_addr_get(void)
{
- return log_buf;
+ return lcb->log_buf;
}
/* Return log buffer size */
u32 log_buf_len_get(void)
{
- return log_buf_len;
+ return lcb->log_buf_len;
}
/* human readable text of the record */
@@ -416,21 +429,21 @@ static char *log_dict(const struct printk_log *msg)
/* get record by index; idx must point to valid msg */
static struct printk_log *log_from_idx(u32 idx)
{
- struct printk_log *msg = (struct printk_log *)(log_buf + idx);
+ struct printk_log *msg = (struct printk_log *)(lcb->log_buf + idx);
/*
* A length == 0 record is the end of buffer marker. Wrap around and
* read the message at the start of the buffer.
*/
if (!msg->len)
- return (struct printk_log *)log_buf;
+ return (struct printk_log *)lcb->log_buf;
return msg;
}
/* get next record; idx must point to valid msg */
static u32 log_next(u32 idx)
{
- struct printk_log *msg = (struct printk_log *)(log_buf + idx);
+ struct printk_log *msg = (struct printk_log *)(lcb->log_buf + idx);
/* length == 0 indicates the end of the buffer; wrap */
/*
@@ -439,7 +452,7 @@ static u32 log_next(u32 idx)
* return the one after that.
*/
if (!msg->len) {
- msg = (struct printk_log *)log_buf;
+ msg = (struct printk_log *)lcb->log_buf;
return msg->len;
}
return idx + msg->len;
@@ -458,10 +471,11 @@ static int logbuf_has_space(u32 msg_size, bool empty)
{
u32 free;
- if (log_next_idx > log_first_idx || empty)
- free = max(log_buf_len - log_next_idx, log_first_idx);
+ if (lcb->log_next_idx > lcb->log_first_idx || empty)
+ free = max(lcb->log_buf_len - lcb->log_next_idx,
+ lcb->log_first_idx);
else
- free = log_first_idx - log_next_idx;
+ free = lcb->log_fir
[RFC PATCH v1 1/2] printk: collect offsets into replaceable structure
Currently, printk relies on several indices that are declared as static
global variables in printk.c. This patch collects those into a single
structure referenced by a pointer. This allows easier replacement of
these indices and pinning to a specific locatino.
Signed-off-by: Sean Hudson
---
kernel/printk/printk.c | 335 +
1 file changed, 172 insertions(+), 163 deletions(-)
diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c
index eea6dbc..7a441f5 100644
--- a/kernel/printk/printk.c
+++ b/kernel/printk/printk.c
@@ -353,28 +353,6 @@ DEFINE_RAW_SPINLOCK(logbuf_lock);
#ifdef CONFIG_PRINTK
DECLARE_WAIT_QUEUE_HEAD(log_wait);
-/* the next printk record to read by syslog(READ) or /proc/kmsg */
-static u64 syslog_seq;
-static u32 syslog_idx;
-static enum log_flags syslog_prev;
-static size_t syslog_partial;
-
-/* index and sequence number of the first record stored in the buffer */
-static u64 log_first_seq;
-static u32 log_first_idx;
-
-/* index and sequence number of the next record to store in the buffer */
-static u64 log_next_seq;
-static u32 log_next_idx;
-
-/* the next printk record to write to the console */
-static u64 console_seq;
-static u32 console_idx;
-static enum log_flags console_prev;
-
-/* the next printk record to read after the last 'clear' command */
-static u64 clear_seq;
-static u32 clear_idx;
#define PREFIX_MAX 32
#define LOG_LINE_MAX (1024 - PREFIX_MAX)
@@ -386,19 +364,54 @@ static u32 clear_idx;
#define LOG_ALIGN __alignof__(struct printk_log)
#define __LOG_BUF_LEN (1 << CONFIG_LOG_BUF_SHIFT)
static char __log_buf[__LOG_BUF_LEN] __aligned(LOG_ALIGN);
-static char *log_buf = __log_buf;
-static u32 log_buf_len = __LOG_BUF_LEN;
+
+/*
+ * This control block collects tracking offsets for the log.
+ */
+struct lcb_t {
+ /* Pointer to log buffer space and length of space */
+ char *log_buf;
+ u32 log_buf_len;
+
+ /* index and sequence of the first record stored in the buffer */
+ u64 log_first_seq;
+ u32 log_first_idx;
+
+ /* index and sequence of the next record to store in the buffer */
+ u64 log_next_seq;
+ u32 log_next_idx;
+
+ /* the next printk record to read by syslog(READ) or /proc/kmsg */
+ u64 syslog_seq;
+ u32 syslog_idx;
+ enum log_flags syslog_prev;
+ size_t syslog_partial;
+
+ /* the next printk record to write to the console */
+ u64 console_seq;
+ u32 console_idx;
+ enum log_flags console_prev;
+
+ /* the next printk record to read after the last 'clear' command */
+ u64 clear_seq;
+ u32 clear_idx;
+} lcb_t;
+
+static struct lcb_t __lcb = {
+ __log_buf, __LOG_BUF_LEN, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0 };
+
+static struct lcb_t *lcb = &__lcb;
/* Return log buffer address */
char *log_buf_addr_get(void)
{
- return log_buf;
+ return lcb->log_buf;
}
/* Return log buffer size */
u32 log_buf_len_get(void)
{
- return log_buf_len;
+ return lcb->log_buf_len;
}
/* human readable text of the record */
@@ -416,21 +429,21 @@ static char *log_dict(const struct printk_log *msg)
/* get record by index; idx must point to valid msg */
static struct printk_log *log_from_idx(u32 idx)
{
- struct printk_log *msg = (struct printk_log *)(log_buf + idx);
+ struct printk_log *msg = (struct printk_log *)(lcb->log_buf + idx);
/*
* A length == 0 record is the end of buffer marker. Wrap around and
* read the message at the start of the buffer.
*/
if (!msg->len)
- return (struct printk_log *)log_buf;
+ return (struct printk_log *)lcb->log_buf;
return msg;
}
/* get next record; idx must point to valid msg */
static u32 log_next(u32 idx)
{
- struct printk_log *msg = (struct printk_log *)(log_buf + idx);
+ struct printk_log *msg = (struct printk_log *)(lcb->log_buf + idx);
/* length == 0 indicates the end of the buffer; wrap */
/*
@@ -439,7 +452,7 @@ static u32 log_next(u32 idx)
* return the one after that.
*/
if (!msg->len) {
- msg = (struct printk_log *)log_buf;
+ msg = (struct printk_log *)lcb->log_buf;
return msg->len;
}
return idx + msg->len;
@@ -458,10 +471,11 @@ static int logbuf_has_space(u32 msg_size, bool empty)
{
u32 free;
- if (log_next_idx > log_first_idx || empty)
- free = max(log_buf_len - log_next_idx, log_first_idx);
+ if (lcb->log_next_idx > lcb->log_first_idx || empty)
+ free = max(lcb->log_buf_len - lcb->log_next_idx,
+ lcb->log_first_idx);
else
- free = log_first_idx - log_next_idx;
+ free = lcb->log_first_idx - lcb->log_next_
[RFC PATCH v1 2/2] printk: external log buffer (CONFIG_LOGBUFFER)
This debug feature provides a convenient way to collect log entries across
multiple, warmboot cycles and to share those entries with a boot loader.
It allows the kernel to use an external buffer for kernel log messages and
is controlled by an optional command line parameter. The buffer can contain
existing log messages from previous boot cycles and/or the bootloader. The
command line parameter was chosen for flexibility, cross arch portability,
and the ability to dynamically enable/disable this feature. The parameter
specifies the address of a control block used to replace the default log
buffer. Existing bootloader and kernel log messages are kept, in order,
inside the new buffer. After a boot that preserves the buffer contents, a
bootloader can display both kernel and bootloader log entries from
multiple, previous boots. It also allows the kernel to display bootloader
log entries along with its own messages.
This feature is intended for debug purposes and has no effect unless the
command line parameter is specified. Further, it validates the passed
control block carefully and if any checks fail, it falls back to the
default behaviour. As such, it can be left enabled by default.
Memory Reservation
--
This feature expects the bootloader to reserve/preserve the shared buffer
memory. This reservation needs to prevent the kernel from overwriting the
external log control block and log entries. In my testing, I've used the
'fdt' commands in uboot to dynamically inject reserved memory regions via
the DT to the kernel.
Based on the initial work of Wolfgang Denk and Igor Lisitsin [1].
Also based on work by: Alexander Streit
<alexander.str...@continental-corporation.com>
[1]
http://git.denx.de/?p=linux-2.6-denx.git;a=commitdiff;h=212f61c7fd3b952a81d1459dd32a86a32ddfd4ce
Signed-off-by: Sean Hudson <sean_hud...@mentor.com>
---
init/Kconfig | 12 +++
init/main.c| 2 +
kernel/printk/printk.c | 267 +++--
3 files changed, 275 insertions(+), 6 deletions(-)
diff --git a/init/Kconfig b/init/Kconfig
index cac3f09..746183b 100644
--- a/init/Kconfig
+++ b/init/Kconfig
@@ -1493,6 +1493,18 @@ config PRINTK
very difficult to diagnose system problems, saying N here is
strongly discouraged.
+config LOGBUFFER
+ bool "External logbuffer" if PRINTK
+ default n
+ depends on PRINTK
+ help
+ This option enables support for an alternative, "external" printk log
+ buffer. If memory contents are preserved, e.g. after a warmboot, this
+ provides a known location for the boot loader to read and display
printk
+ entries from the kernel. If desired, the bootloader can write its own
+ log entries which the kernel will display with its own log entries.
+ Further, this capability can be used across multiple warmboot cycles.
+
config PRINTK_NMI
def_bool y
depends on PRINTK
diff --git a/init/main.c b/init/main.c
index a8a58e2..4a5913c 100644
--- a/init/main.c
+++ b/init/main.c
@@ -93,6 +93,7 @@ static int kernel_init(void *);
extern void init_IRQ(void);
extern void fork_init(void);
extern void radix_tree_init(void);
+extern void setup_ext_logbuff(void);
/*
* Debug helper: via this flag we know that we are in 'early bootup code'
@@ -535,6 +536,7 @@ asmlinkage __visible void __init start_kernel(void)
sort_main_extable();
trap_init();
mm_init();
+ setup_ext_logbuff();
/*
* Set up the scheduler prior starting any interrupts (such as the
diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c
index 7a441f5..017b4d4 100644
--- a/kernel/printk/printk.c
+++ b/kernel/printk/printk.c
@@ -331,17 +331,17 @@ enum log_flags {
};
struct printk_log {
- u64 ts_nsec;/* timestamp in nanoseconds */
+#ifdef CONFIG_LOGBUFFER
+ u32 log_magic; /* sanity check number */
+#endif
u16 len;/* length of entire record */
u16 text_len; /* length of text buffer */
u16 dict_len; /* length of dictionary buffer */
u8 facility;/* syslog facility */
u8 flags:5; /* internal record flags */
u8 level:3; /* syslog level */
+ u64 ts_nsec;/* timestamp in nanoseconds */
}
-#ifdef CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS
-__packed __aligned(4)
-#endif
;
/*
@@ -366,7 +366,14 @@ DECLARE_WAIT_QUEUE_HEAD(log_wait);
static char __log_buf[__LOG_BUF_LEN] __aligned(LOG_ALIGN);
/*
- * This control block collects tracking offsets for the log.
+ * This control block collects tracking offsets for the log into a single
+ * place. It also facilitates pointing the log to another location, and,
+ * when combined with the CONFIG_LOGBUFFER feature, it allows log sharing
+ * between the bootloader and the kernel.
+ *
+ * NOTE:
[RFC PATCH v1 0/2] printk: Shared kernel logging
This patch set is based on Linus' v4.8-rc8 tag. This debug feature allows the kernel to use an external buffer and control block for kernel log messages. The feature is controlled by an optional command line parameter. The existing buffer and control block can contain existing log messages from previous boot cycles and/or the bootloader. The command line parameter was chosen for flexibility, cross arch portability, and the ability to dynamically enable/disable this feature. The parameter specifies the address of a control block used to replace the default log buffer. Existing bootloader and kernel log messages are kept, in order, inside the new buffer. After a boot that preserves the buffer contents, a bootloader can display both kernel and bootloader log entries from multiple, previous boots. It also allows the kernel to display bootloader log entries along with its own messages. This feature is intended for debug purposes and has no effect unless the command line parameter is specified. Further, it validates the passed control block carefully and if any checks fail, it falls back to the default behaviour. As such, it can be left enabled by default. Memory Reservation This feature expects the bootloader to reserve/preserve the shared buffer memory. This reservation needs to prevent the kernel from overwriting the external log control block and log entries. In my testing, I've used the 'fdt' commands in uboot to dynamically inject reserved memory regions via the DT to the kernel. Sean Hudson (2): printk: collect offsets into replaceable structure printk: external log buffer (CONFIG_LOGBUFFER) init/Kconfig | 12 + init/main.c| 2 + kernel/printk/printk.c | 598 +++-- 3 files changed, 445 insertions(+), 167 deletions(-) -- 1.9.1
[RFC PATCH v1 2/2] printk: external log buffer (CONFIG_LOGBUFFER)
This debug feature provides a convenient way to collect log entries across
multiple, warmboot cycles and to share those entries with a boot loader.
It allows the kernel to use an external buffer for kernel log messages and
is controlled by an optional command line parameter. The buffer can contain
existing log messages from previous boot cycles and/or the bootloader. The
command line parameter was chosen for flexibility, cross arch portability,
and the ability to dynamically enable/disable this feature. The parameter
specifies the address of a control block used to replace the default log
buffer. Existing bootloader and kernel log messages are kept, in order,
inside the new buffer. After a boot that preserves the buffer contents, a
bootloader can display both kernel and bootloader log entries from
multiple, previous boots. It also allows the kernel to display bootloader
log entries along with its own messages.
This feature is intended for debug purposes and has no effect unless the
command line parameter is specified. Further, it validates the passed
control block carefully and if any checks fail, it falls back to the
default behaviour. As such, it can be left enabled by default.
Memory Reservation
--
This feature expects the bootloader to reserve/preserve the shared buffer
memory. This reservation needs to prevent the kernel from overwriting the
external log control block and log entries. In my testing, I've used the
'fdt' commands in uboot to dynamically inject reserved memory regions via
the DT to the kernel.
Based on the initial work of Wolfgang Denk and Igor Lisitsin [1].
Also based on work by: Alexander Streit
[1]
http://git.denx.de/?p=linux-2.6-denx.git;a=commitdiff;h=212f61c7fd3b952a81d1459dd32a86a32ddfd4ce
Signed-off-by: Sean Hudson
---
init/Kconfig | 12 +++
init/main.c| 2 +
kernel/printk/printk.c | 267 +++--
3 files changed, 275 insertions(+), 6 deletions(-)
diff --git a/init/Kconfig b/init/Kconfig
index cac3f09..746183b 100644
--- a/init/Kconfig
+++ b/init/Kconfig
@@ -1493,6 +1493,18 @@ config PRINTK
very difficult to diagnose system problems, saying N here is
strongly discouraged.
+config LOGBUFFER
+ bool "External logbuffer" if PRINTK
+ default n
+ depends on PRINTK
+ help
+ This option enables support for an alternative, "external" printk log
+ buffer. If memory contents are preserved, e.g. after a warmboot, this
+ provides a known location for the boot loader to read and display
printk
+ entries from the kernel. If desired, the bootloader can write its own
+ log entries which the kernel will display with its own log entries.
+ Further, this capability can be used across multiple warmboot cycles.
+
config PRINTK_NMI
def_bool y
depends on PRINTK
diff --git a/init/main.c b/init/main.c
index a8a58e2..4a5913c 100644
--- a/init/main.c
+++ b/init/main.c
@@ -93,6 +93,7 @@ static int kernel_init(void *);
extern void init_IRQ(void);
extern void fork_init(void);
extern void radix_tree_init(void);
+extern void setup_ext_logbuff(void);
/*
* Debug helper: via this flag we know that we are in 'early bootup code'
@@ -535,6 +536,7 @@ asmlinkage __visible void __init start_kernel(void)
sort_main_extable();
trap_init();
mm_init();
+ setup_ext_logbuff();
/*
* Set up the scheduler prior starting any interrupts (such as the
diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c
index 7a441f5..017b4d4 100644
--- a/kernel/printk/printk.c
+++ b/kernel/printk/printk.c
@@ -331,17 +331,17 @@ enum log_flags {
};
struct printk_log {
- u64 ts_nsec;/* timestamp in nanoseconds */
+#ifdef CONFIG_LOGBUFFER
+ u32 log_magic; /* sanity check number */
+#endif
u16 len;/* length of entire record */
u16 text_len; /* length of text buffer */
u16 dict_len; /* length of dictionary buffer */
u8 facility;/* syslog facility */
u8 flags:5; /* internal record flags */
u8 level:3; /* syslog level */
+ u64 ts_nsec;/* timestamp in nanoseconds */
}
-#ifdef CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS
-__packed __aligned(4)
-#endif
;
/*
@@ -366,7 +366,14 @@ DECLARE_WAIT_QUEUE_HEAD(log_wait);
static char __log_buf[__LOG_BUF_LEN] __aligned(LOG_ALIGN);
/*
- * This control block collects tracking offsets for the log.
+ * This control block collects tracking offsets for the log into a single
+ * place. It also facilitates pointing the log to another location, and,
+ * when combined with the CONFIG_LOGBUFFER feature, it allows log sharing
+ * between the bootloader and the kernel.
+ *
+ * NOTE:
+ * By convention, the control block and the log buffer are contiguous.
+ *
[RFC PATCH v1 0/2] printk: Shared kernel logging
This patch set is based on Linus' v4.8-rc8 tag. This debug feature allows the kernel to use an external buffer and control block for kernel log messages. The feature is controlled by an optional command line parameter. The existing buffer and control block can contain existing log messages from previous boot cycles and/or the bootloader. The command line parameter was chosen for flexibility, cross arch portability, and the ability to dynamically enable/disable this feature. The parameter specifies the address of a control block used to replace the default log buffer. Existing bootloader and kernel log messages are kept, in order, inside the new buffer. After a boot that preserves the buffer contents, a bootloader can display both kernel and bootloader log entries from multiple, previous boots. It also allows the kernel to display bootloader log entries along with its own messages. This feature is intended for debug purposes and has no effect unless the command line parameter is specified. Further, it validates the passed control block carefully and if any checks fail, it falls back to the default behaviour. As such, it can be left enabled by default. Memory Reservation This feature expects the bootloader to reserve/preserve the shared buffer memory. This reservation needs to prevent the kernel from overwriting the external log control block and log entries. In my testing, I've used the 'fdt' commands in uboot to dynamically inject reserved memory regions via the DT to the kernel. Sean Hudson (2): printk: collect offsets into replaceable structure printk: external log buffer (CONFIG_LOGBUFFER) init/Kconfig | 12 + init/main.c| 2 + kernel/printk/printk.c | 598 +++-- 3 files changed, 445 insertions(+), 167 deletions(-) -- 1.9.1
