linux-kernel - Re: [PATCH v2 1/2] printk: Fix panic caused by passing log_buf

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <20180918224312.6e9aef50@vmware.local.home>
Date:   Tue, 18 Sep 2018 22:43:12 -0400
From:   Steven Rostedt <rostedt@...dmis.org>
To:     Sergey Senozhatsky <sergey.senozhatsky.work@...il.com>
Cc:     He Zhe <zhe.he@...driver.com>, pmladek@...e.com,
        sergey.senozhatsky@...il.com, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2 1/2] printk: Fix panic caused by passing log_buf_len
 to command line

On Wed, 19 Sep 2018 11:39:32 +0900
Sergey Senozhatsky <sergey.senozhatsky.work@...il.com> wrote:

> On (09/19/18 10:27), He Zhe wrote:
> > On 2018年09月19日 09:50, Sergey Senozhatsky wrote:  
> > > On (09/19/18 01:17), zhe.he@...driver.com wrote:  
> > >> @@ -1048,7 +1048,14 @@ static void __init log_buf_len_update(unsigned size)
> > >>  /* save requested log_buf_len since it's too early to process it */
> > >>  static int __init log_buf_len_setup(char *str)
> > >>  {
> > >> -	unsigned size = memparse(str, &str);
> > >> +	unsigned size;  
> > > 	unsigned int size;  
> > 
> > This is in v1 but then Steven suggested that it should be split out
> > and only keep the pure fix part here.  
> 
> Ah, I see.
> 
> Hmm... memparse() returns u64 value. A user *probably* can ask the kernel
> to allocate log_buf larger than 'unsigned int'.
> 
> So may be I'd do two fixes here:
> 
>  First  - switch to u64 size.
>  Second - check for NULL str.
> 
> 
> Steven, Petr, what do you think?
> 

I think I would switch it around. Check for NULL first, and then switch
to u64. It was always an int, do we need to backport converting it to
u64 to stable? The NULL check is a definite, the overflow of int
shouldn't crash anything.

-- Steve