Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753114AbbETMKu (ORCPT ); Wed, 20 May 2015 08:10:50 -0400 Received: from cantor2.suse.de ([195.135.220.15]:42266 "EHLO mx2.suse.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751609AbbETMKs (ORCPT ); Wed, 20 May 2015 08:10:48 -0400 Date: Wed, 20 May 2015 14:10:44 +0200 From: Petr Mladek To: Marcin Niesluchowski Cc: Andrew Morton , Jan Kara , "Steven Rostedt (Red Hat)" , Alex Elder , "Luis R. Rodriguez" , Peter Hurley , Joe Perches , Kay Sievers , linux-kernel@vger.kernel.org, =?utf-8?Q?=C5=81ukasz?= Stelmach , Karol Lewandowski , Tejun Heo Subject: Re: [PATCH] printk: Remove possible overflow in user read buffer Message-ID: <20150520121044.GA2728@pathway.suse.cz> References: <1432119588-22209-1-git-send-email-m.niesluchow@samsung.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1432119588-22209-1-git-send-email-m.niesluchow@samsung.com> User-Agent: Mutt/1.5.21 (2010-09-15) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2665 Lines: 89 On Wed 2015-05-20 12:59:48, Marcin Niesluchowski wrote: > Reading message with dict may cause user buffer overflow due to > no limits of written dict and hardcoded user read buffer size. > As limits of dict are not clear, it may be possible in extreme > use case to trigger it (e.g. by driver passing some dict parameters > from userland or other module logging large key-value data). > > Truncate dict read by user when its size would cause user read > buffer to overflow. > > Bug was found during work on extension of kmsg enabling writing > dict from userspace. By coincidence, Tejun has already provided slightly different patch for this problem, see https://lkml.org/lkml/2015/5/14/494 AFAIK, Tejun's patch already is in the -mm tree and thus on the way to get merged. I would prefer Tejun's solution. Best Regards, Petr > > Signed-off-by: Marcin Niesluchowski > --- > kernel/printk/printk.c | 11 ++++++++++- > 1 file changed, 10 insertions(+), 1 deletion(-) > > diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c > index c099b08..b61602d 100644 > --- a/kernel/printk/printk.c > +++ b/kernel/printk/printk.c > @@ -505,6 +505,7 @@ int check_syslog_permissions(int type, bool from_file) > return security_syslog(type); > } > > +#define USER_READ_LOG_BUF_LEN 8192 > > /* /dev/kmsg - userspace message inject/listen interface */ > struct devkmsg_user { > @@ -512,7 +513,7 @@ struct devkmsg_user { > u32 idx; > enum log_flags prev; > struct mutex lock; > - char buf[8192]; > + char buf[USER_READ_LOG_BUF_LEN]; > }; > > static ssize_t devkmsg_write(struct kiocb *iocb, struct iov_iter *from) > @@ -648,21 +649,29 @@ static ssize_t devkmsg_read(struct file *file, char __user *buf, > unsigned char c = log_dict(msg)[i]; > > if (line) { > + if (len >= USER_READ_LOG_BUF_LEN-1) > + break; > user->buf[len++] = ' '; > line = false; > } > > if (c == '\0') { > + if (len >= USER_READ_LOG_BUF_LEN-1) > + break; > user->buf[len++] = '\n'; > line = true; > continue; > } > > if (c < ' ' || c >= 127 || c == '\\') { > + if (len >= USER_READ_LOG_BUF_LEN-4) > + break; > len += sprintf(user->buf + len, "\\x%02x", c); > continue; > } > > + if (len >= USER_READ_LOG_BUF_LEN-1) > + break; > user->buf[len++] = c; > } > user->buf[len++] = '\n'; > -- > 1.9.1 > -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/