Re: [RFC PATCH v1 2/2] printk: external log buffer (CONFIG_LOGBUFFER)

[Petr Mladek <pmladek@suse.com>]

On Fri 2016-09-30 23:06:49, Sean Hudson wrote:
> On 9/30/2016 7:39 AM, Petr Mladek wrote:
> > On Thu 2016-09-29 19:55:56, Sean Hudson wrote:
> >> +	/* Overwrite the default lcb pointer, buffer pointer, and length. */
> >> +	lcb = new_lcb;
> >> +	lcb->log_buf = (char *)new_lcb->log_buf;
> >> +	lcb->log_buf_len = new_lcb->log_buf_len;
> > 
> > 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?


> > 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...


>  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.

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.

Also you did not answered the question about your plans. I wonder
which bootloader would be able to use such a feature.

Best Regards,
Petr