Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S968253Ab3HJAtg (ORCPT ); Fri, 9 Aug 2013 20:49:36 -0400 Received: from mail-pa0-f46.google.com ([209.85.220.46]:41797 "EHLO mail-pa0-f46.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S968075Ab3HJAtf (ORCPT ); Fri, 9 Aug 2013 20:49:35 -0400 MIME-Version: 1.0 In-Reply-To: <1376080406-4r7r3uye-mutt-n-horiguchi@ah.jp.nec.com> References: <1376080406-4r7r3uye-mutt-n-horiguchi@ah.jp.nec.com> Date: Sat, 10 Aug 2013 08:49:34 +0800 Message-ID: Subject: Re: [PATCH 1/1] pagemap: fix buffer overflow in add_page_map() From: yonghua zheng To: Naoya Horiguchi Cc: linux-kernel@vger.kernel.org, linux-mm@kvack.org, Motohiro KOSAKI Content-Type: text/plain; charset=UTF-8 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 3739 Lines: 117 Update the patch according to Naoya's comment, I also run ./scripts/checkpatch.pl, and it passed ;D. >From 96826b0fdf9ec6d6e16c2c595f371dbb841250f7 Mon Sep 17 00:00:00 2001 From: Yonghua Zheng Date: Mon, 5 Aug 2013 12:12:24 +0800 Subject: [PATCH 1/1] pagemap: fix buffer overflow in add_to_pagemap() In struc pagemapread: struct pagemapread { int pos, len; pagemap_entry_t *buffer; bool v2; }; pos is number of PM_ENTRY_BYTES in buffer, but len is the size of buffer, it is a mistake to compare pos and len in add_to_pagemap() for checking buffer is full or not, and this can lead to buffer overflow and random kernel panic issue. Correct len to be total number of PM_ENTRY_BYTES in buffer. Signed-off-by: Yonghua Zheng diff --git a/fs/proc/task_mmu.c b/fs/proc/task_mmu.c index dbf61f6..cb98853 100644 --- a/fs/proc/task_mmu.c +++ b/fs/proc/task_mmu.c @@ -1116,8 +1116,8 @@ static ssize_t pagemap_read(struct file *file, char __user *buf, goto out_task; pm.v2 = soft_dirty_cleared; - pm.len = PM_ENTRY_BYTES * (PAGEMAP_WALK_SIZE >> PAGE_SHIFT); - pm.buffer = kmalloc(pm.len, GFP_TEMPORARY); + pm.len = (PAGEMAP_WALK_SIZE >> PAGE_SHIFT); + pm.buffer = kmalloc(pm.len * PM_ENTRY_BYTES, GFP_TEMPORARY); ret = -ENOMEM; if (!pm.buffer) goto out_task; -- 1.7.9.5 On Sat, Aug 10, 2013 at 4:33 AM, Naoya Horiguchi wrote: > On Fri, Aug 09, 2013 at 01:16:41PM +0800, yonghua zheng wrote: >> Hi, >> >> Recently we met quite a lot of random kernel panic issues after enable >> CONFIG_PROC_PAGE_MONITOR in kernel, after debuggint sometime we found >> this has something to do with following bug in pagemap: >> >> In struc pagemapread: >> >> struct pagemapread { >> int pos, len; >> pagemap_entry_t *buffer; >> bool v2; >> }; >> >> pos is number of PM_ENTRY_BYTES in buffer, but len is the size of buffer, >> it is a mistake to compare pos and len in add_page_map() for checking > > s/add_page_map/add_to_pagemap/ ? > >> buffer is full or not, and this can lead to buffer overflow and random >> kernel panic issue. >> >> Correct len to be total number of PM_ENTRY_BYTES in buffer. >> >> Signed-off-by: Yonghua Zheng > > You can find coding style violation with scripts/checkpatch.pl. > And I think this patch is worth going into -stable trees > (maybe since 2.6.34.) > > The fix itself looks fine to me. > > Thanks, > Naoya Horiguchi > >> --- >> fs/proc/task_mmu.c | 4 ++-- >> 1 file changed, 2 insertions(+), 2 deletions(-) >> >> diff --git a/fs/proc/task_mmu.c b/fs/proc/task_mmu.c >> index dbf61f6..cb98853 100644 >> --- a/fs/proc/task_mmu.c >> +++ b/fs/proc/task_mmu.c >> @@ -1116,8 +1116,8 @@ static ssize_t pagemap_read(struct file *file, >> char __user *buf, >> goto out_task; >> >> pm.v2 = soft_dirty_cleared; >> - pm.len = PM_ENTRY_BYTES * (PAGEMAP_WALK_SIZE >> PAGE_SHIFT); >> - pm.buffer = kmalloc(pm.len, GFP_TEMPORARY); >> + pm.len = (PAGEMAP_WALK_SIZE >> PAGE_SHIFT); >> + pm.buffer = kmalloc(pm.len * PM_ENTRY_BYTES, GFP_TEMPORARY); >> ret = -ENOMEM; >> if (!pm.buffer) >> goto out_task; >> >> -- >> 1.7.9.5 >> >> -- >> To unsubscribe, send a message with 'unsubscribe linux-mm' in >> the body to majordomo@kvack.org. For more info on Linux MM, >> see: http://www.linux-mm.org/ . >> Don't email: email@kvack.org >> -- 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/