From patchwork Thu Jul 19 23:43:41 2012 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Anton Vorontsov X-Patchwork-Id: 10154 Return-Path: X-Original-To: patchwork@peony.canonical.com Delivered-To: patchwork@peony.canonical.com Received: from fiordland.canonical.com (fiordland.canonical.com [91.189.94.145]) by peony.canonical.com (Postfix) with ESMTP id C378523F08 for ; Thu, 19 Jul 2012 23:46:10 +0000 (UTC) Received: from mail-yx0-f180.google.com (mail-yx0-f180.google.com [209.85.213.180]) by fiordland.canonical.com (Postfix) with ESMTP id 80058A18119 for ; Thu, 19 Jul 2012 23:46:10 +0000 (UTC) Received: by yenq6 with SMTP id q6so3726680yen.11 for ; Thu, 19 Jul 2012 16:46:10 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20120113; h=x-forwarded-to:x-forwarded-for:delivered-to:received-spf:from:to:cc :subject:date:message-id:x-mailer:in-reply-to:references :x-gm-message-state; bh=NENQAFU7SDcHCGi1Pck+1dyZfQFgY6lvKgkCgZAhnFA=; b=i9mT8p15ntWWmR1urJSWkznzJnnJnRFrIxgsZM6k/AWXCewa1ZZg4qfAopI55uSNN3 Tgewo8Ad/LiWmWr8roiAIfHHIh4D6e/ywGs7B6kKPPs4lBslcwICMWQhN/VkZgvDqfUk WqsUEWc0DN440wqcSaLgXJ5uHGlcw7pTjJUceXiUPcJBlp0NjIwMOwO4bh6+xYi/Ieta pcpD4DstkuolikmpPXC5ESECWQfXuY7NysNJks6BiLyD15BQzXAxHU+AQpoBVnlFksAG htSQJbo99jKZI/5xpRA5ugnxgIWXXujcfXIaZAZEUdJP5PvFF/zqaMQw7gNEc2Svf6Gh F5Ww== Received: by 10.43.132.197 with SMTP id hv5mr2363259icc.33.1342741569849; Thu, 19 Jul 2012 16:46:09 -0700 (PDT) X-Forwarded-To: linaro-patchwork@canonical.com X-Forwarded-For: patch@linaro.org linaro-patchwork@canonical.com Delivered-To: patches@linaro.org Received: by 10.231.153.7 with SMTP id i7csp17022ibw; Thu, 19 Jul 2012 16:46:08 -0700 (PDT) Received: by 10.236.79.6 with SMTP id h6mr4052806yhe.71.1342741568319; Thu, 19 Jul 2012 16:46:08 -0700 (PDT) Received: from mail-gh0-f178.google.com (mail-gh0-f178.google.com [209.85.160.178]) by mx.google.com with ESMTPS id d45si3429436yhe.27.2012.07.19.16.46.08 (version=TLSv1/SSLv3 cipher=OTHER); Thu, 19 Jul 2012 16:46:08 -0700 (PDT) Received-SPF: neutral (google.com: 209.85.160.178 is neither permitted nor denied by best guess record for domain of anton.vorontsov@linaro.org) client-ip=209.85.160.178; Authentication-Results: mx.google.com; spf=neutral (google.com: 209.85.160.178 is neither permitted nor denied by best guess record for domain of anton.vorontsov@linaro.org) smtp.mail=anton.vorontsov@linaro.org Received: by ghbf1 with SMTP id f1so4073266ghb.37 for ; Thu, 19 Jul 2012 16:46:08 -0700 (PDT) Received: by 10.66.72.225 with SMTP id g1mr7378611pav.3.1342741567569; Thu, 19 Jul 2012 16:46:07 -0700 (PDT) Received: from localhost (m940536d0.tmodns.net. [208.54.5.148]) by mx.google.com with ESMTPS id nu5sm2672927pbb.53.2012.07.19.16.46.02 (version=TLSv1/SSLv3 cipher=OTHER); Thu, 19 Jul 2012 16:46:06 -0700 (PDT) From: Anton Vorontsov To: Greg Kroah-Hartman , Kees Cook , Colin Cross , Tony Luck Cc: Dan Carpenter , John Stultz , arve@android.com, Rebecca Schultz Zavin , Marco Stornelli , linux-kernel@vger.kernel.org, devel@driverdev.osuosl.org, linaro-kernel@lists.linaro.org, patches@linaro.org, kernel-team@android.com Subject: [PATCH] pstore/ram: Fix possible NULL dereference Date: Thu, 19 Jul 2012 16:43:41 -0700 Message-Id: <1342741421-17956-1-git-send-email-anton.vorontsov@linaro.org> X-Mailer: git-send-email 1.7.10.4 In-Reply-To: <20120719142856.GA25184@elgon.mountain> References: <20120719142856.GA25184@elgon.mountain> X-Gm-Message-State: ALoCoQkkGrOYyqwXA5PxwzFwWPmfrHo43OkXtPnCT7FJAoIjoAUG4g9hOZBWIu7mvRL/VWgU7mcK We can dereference 'cxt->cprz' if console and dump logging are disabled (which is unlikely, but still possible to do). This patch fixes the issue by changing the code so that we don't dereference przs at all, we can just calculate bufsize from console_size and record_size values. Plus, while at it, the patch improves the buffer size calculation. After Kay's printk rework, we know the optimal buffer size for console logging -- it is LOG_LINE_MAX (defined privately in printk.c). Previously, if only console logging was enabled, we would allocate unnecessary large buffer in pstore, while we only need LOG_LINE_MAX. (Pstore console logging is still capable of handling buffers > LOG_LINE_MAX, it will just do multiple calls to psinfo->write). Note that I don't export the constant, since we will do even a better thing soon: we will switch console logging to a new write_buf API, which will eliminate the need for the additional buffer; and so we won't need the constant. Reported-by: Dan Carpenter Signed-off-by: Anton Vorontsov Acked-by: Kees Cook --- fs/pstore/ram.c | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/fs/pstore/ram.c b/fs/pstore/ram.c index b86b2b7..c34fccf 100644 --- a/fs/pstore/ram.c +++ b/fs/pstore/ram.c @@ -414,13 +414,14 @@ static int __devinit ramoops_probe(struct platform_device *pdev) cxt->pstore.data = cxt; /* - * Console can handle any buffer size, so prefer dumps buffer - * size since usually it is smaller. + * Console can handle any buffer size, so prefer LOG_LINE_MAX. If we + * have to handle dumps, we must have at least record_size buffer. And + * for ftrace, bufsize is irrelevant (if bufsize is 0, buf will be + * ZERO_SIZE_PTR). */ - if (cxt->przs) - cxt->pstore.bufsize = cxt->przs[0]->buffer_size; - else - cxt->pstore.bufsize = cxt->cprz->buffer_size; + if (cxt->console_size) + cxt->pstore.bufsize = 1024; /* LOG_LINE_MAX */ + cxt->pstore.bufsize = max(cxt->record_size, cxt->pstore.bufsize); cxt->pstore.buf = kmalloc(cxt->pstore.bufsize, GFP_KERNEL); spin_lock_init(&cxt->pstore.buf_lock); if (!cxt->pstore.buf) {