linux-kernel - Re: [PATCH v2 1/2] printk: Add function to return log buffer address and size

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <53DB4C9A.8050400@linux.vnet.ibm.com>
Date:	Fri, 01 Aug 2014 13:45:22 +0530
From:	Vasant Hegde <hegdevasant@...ux.vnet.ibm.com>
To:	Andrew Morton <akpm@...ux-foundation.org>
CC:	linux-kernel@...r.kernel.org, linuxppc-dev@...ts.ozlabs.org,
	Linus Torvalds <torvalds@...ux-foundation.org>,
	Benjamin Herrenschmidt <benh@...nel.crashing.org>
Subject: Re: [PATCH v2 1/2] printk: Add function to return log buffer address
 and size

On 08/01/2014 03:52 AM, Andrew Morton wrote:
> On Thu, 31 Jul 2014 21:05:48 +0530 Vasant Hegde <hegdevasant@...ux.vnet.ibm.com> wrote:
>
>> Platforms like IBM Power Systems supports service processor
>> assisted dump. It provides interface to add memory region to
>> be captured when system is crashed.
>>
>> During initialization/running we can add kernel memory region
>> to be collected.
>>
>> Presently we don't have a way to get the log buffer base address
>> and size. This patch adds support to return log buffer address
>> and size.
>>
>> ...
>>
>> --- a/include/linux/printk.h
>> +++ b/include/linux/printk.h
>> @@ -10,6 +10,9 @@
>>   extern const char linux_banner[];
>>   extern const char linux_proc_banner[];
>>
>> +extern void *get_log_buf_addr(void);
>> +extern u32 get_log_buf_len(void);
>> +
>>   static inline int printk_get_level(const char *buffer)
>>   {
>>   	if (buffer[0] == KERN_SOH_ASCII && buffer[1]) {
>> diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c
>> index 13e839d..4049f7b 100644
>> --- a/kernel/printk/printk.c
>> +++ b/kernel/printk/printk.c
>> @@ -270,6 +270,18 @@ static char __log_buf[__LOG_BUF_LEN] __aligned(LOG_ALIGN);
>>   static char *log_buf = __log_buf;
>>   static u32 log_buf_len = __LOG_BUF_LEN;
>>
>> +/* Return log buffer address */
>> +void *get_log_buf_addr(void)
>> +{
>> +	return log_buf;
>> +}
>> +
>> +/* Return log buffer size */
>> +u32 get_log_buf_len(void)
>> +{
>> +	return log_buf_len;
>> +}
>> +
>>   /* human readable text of the record */
>>   static char *log_text(const struct printk_log *msg)
>>   {
>

Andrew,

Thanks for the review.. Shall I add your Ack?

> Looks OK to me, although I think log_buf_addr_get() and
> log_buf_len_get() would be better names.  The kernel uses some
> big-endian names and some little-endian-names and some middle-endian
> names.  It's all rather a mess.  These symbols are already big-endian
> (ie: decreasing significance):
>
> 	log_buf -> addr -> get
> 	log_buf -> len -> get

Sure . Will rename function as above.

>
>
> The world wouldn't end if we simply made log_buf and log_buf_len global
> symbols.  The pros and cons are all minor.
>
> It's probably better to make get_log_buf_addr() tell the truth and
> return a char *.
>
>
> Please include this in whatever tree carries "powerpc/powernv:
> Interface to register/unregister opal dump region".

Sure.. Will work with BenH.

-Vasant

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/