Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1756449Ab2FOMKi (ORCPT ); Fri, 15 Jun 2012 08:10:38 -0400 Received: from mail-wg0-f44.google.com ([74.125.82.44]:44750 "EHLO mail-wg0-f44.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754633Ab2FOMKh (ORCPT ); Fri, 15 Jun 2012 08:10:37 -0400 Date: Fri, 15 Jun 2012 14:10:31 +0200 From: Ingo Molnar To: Srikar Dronamraju Cc: Oleg Nesterov , Hugh Dickins , Ingo Molnar , Peter Zijlstra , Ananth N Mavinakayanahalli , Anton Arapov , Linus Torvalds , Masami Hiramatsu , linux-kernel@vger.kernel.org Subject: Re: [PATCH 2/3] uprobes: __copy_insn() should ensure a_ops->readpage != NULL Message-ID: <20120615121031.GA28541@gmail.com> References: <20120607165942.GA31966@redhat.com> <20120607170018.GB31974@redhat.com> <20120615062534.GB3811@linux.vnet.ibm.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20120615062534.GB3811@linux.vnet.ibm.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: 1626 Lines: 46 * Srikar Dronamraju wrote: > * Oleg Nesterov [2012-06-07 19:00:18]: > > > __copy_insn() blindly calls read_mapping_page(), this will crash > > the kernel if ->readpage == NULL, add the necessary check. For > > example, hugetlbfs_aops->readpage is NULL. Perhaps we should change > > read_mapping_page() instead. > > > > Signed-off-by: Oleg Nesterov > > --- > > kernel/events/uprobes.c | 2 ++ > > 1 files changed, 2 insertions(+), 0 deletions(-) > > > > diff --git a/kernel/events/uprobes.c b/kernel/events/uprobes.c > > index 48d53af..9c53bc2 100644 > > --- a/kernel/events/uprobes.c > > +++ b/kernel/events/uprobes.c > > @@ -616,6 +616,8 @@ __copy_insn(struct address_space *mapping, struct vm_area_struct *vma, char *ins > > > > if (!filp) > > return -EINVAL; > > + if (!mapping->a_ops->readpage) > > + return -EIO; > > Nit: Should there be a blank line before the if. Ingo had insisted on > these coding style changes. Well, within sensible limits: if()'s can certainly be in a block if they do related things (such as boundary checks, etc.), so the above isn't ugly IMO. Nor are separating newlines strictly necessary - it's also a function of the complexity of the code in question - if it's complex code then we want all visual and cleanliness help that we can think of. Thanks, Ingo -- 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/