From mboxrd@z Thu Jan 1 00:00:00 1970 From: James Chapman Date: Mon, 12 May 2008 17:12:38 +0000 Subject: Re: [PATCH 6/6] drivers/net/pppol2tp.c: remove null pointer dereference Message-Id: <48287A86.1070401@katalix.com> List-Id: References: In-Reply-To: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: Julia Lawall Cc: linux-kernel@vger.kernel.org, kernel-janitors@vger.kernel.org, netdev Adding netdev to CC list. Julia Lawall wrote: > From: Julia Lawall > > If session is NULL, it is not possible to access its name field. So I have > split apart the printing of the error message to drop the printing of the > name field in this case. I suggest add a note in the patch description that this bug will only be hit if the driver's debug is enabled. > This problem was found using the following semantic match > (http://www.emn.fr/x-info/coccinelle/) > > // > @@ > expression E, E1; > identifier f; > statement S1,S2,S3; > @@ > > * if (E = NULL) > { > ... when != if (E = NULL) S1 else S2 > when != E = E1 > * E->f > ... when any > return ...; > } > else S3 > // Perhaps the above text should be in the additional info section of the patch description? Since this is a network driver, can you resubmit the patch to netdev? > Signed-off-by: Julia Lawall > > --- > > diff -u -p a/drivers/net/pppol2tp.c b/drivers/net/pppol2tp.c > --- linux-2.6/drivers/net/pppol2tp.c 2008-05-09 16:46:57.000000000 +0200 > +++ linuxcopy/drivers/net/pppol2tp.c 2008-05-12 15:30:52.000000000 +0200 > @@ -1621,9 +1621,16 @@ out_no_ppp: > end: > release_sock(sk); > > - if (error != 0) > - PRINTK(session ? session->debug : -1, PPPOL2TP_MSG_CONTROL, KERN_WARNING, > - "%s: connect failed: %d\n", session->name, error); > + if (error != 0) { > + if (session) > + PRINTK(session->debug, > + PPPOL2TP_MSG_CONTROL, KERN_WARNING, > + "%s: connect failed: %d\n", > + session->name, error); > + else > + PRINTK(-1, PPPOL2TP_MSG_CONTROL, KERN_WARNING, > + "connect failed: %d\n", error); > + } > > return error; > } > From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1757470AbYELRMu (ORCPT ); Mon, 12 May 2008 13:12:50 -0400 Received: (majordomo@vger.kernel.org) by vger.kernel.org id S1757018AbYELRMk (ORCPT ); Mon, 12 May 2008 13:12:40 -0400 Received: from s36.avahost.net ([74.53.95.194]:36845 "EHLO s36.avahost.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753124AbYELRMj (ORCPT ); Mon, 12 May 2008 13:12:39 -0400 Message-ID: <48287A86.1070401@katalix.com> Date: Mon, 12 May 2008 18:12:38 +0100 From: James Chapman Organization: Katalix Systems Ltd User-Agent: Thunderbird 2.0.0.14 (Windows/20080421) MIME-Version: 1.0 To: Julia Lawall CC: linux-kernel@vger.kernel.org, kernel-janitors@vger.kernel.org, netdev Subject: Re: [PATCH 6/6] drivers/net/pppol2tp.c: remove null pointer dereference References: In-Reply-To: X-Enigmail-Version: 0.95.6 Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit X-AntiAbuse: This header was added to track abuse, please include it with any abuse report X-AntiAbuse: Primary Hostname - s36.avahost.net X-AntiAbuse: Original Domain - vger.kernel.org X-AntiAbuse: Originator/Caller UID/GID - [47 12] / [47 12] X-AntiAbuse: Sender Address Domain - katalix.com Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Adding netdev to CC list. Julia Lawall wrote: > From: Julia Lawall > > If session is NULL, it is not possible to access its name field. So I have > split apart the printing of the error message to drop the printing of the > name field in this case. I suggest add a note in the patch description that this bug will only be hit if the driver's debug is enabled. > This problem was found using the following semantic match > (http://www.emn.fr/x-info/coccinelle/) > > // > @@ > expression E, E1; > identifier f; > statement S1,S2,S3; > @@ > > * if (E == NULL) > { > ... when != if (E == NULL) S1 else S2 > when != E = E1 > * E->f > ... when any > return ...; > } > else S3 > // Perhaps the above text should be in the additional info section of the patch description? Since this is a network driver, can you resubmit the patch to netdev? > Signed-off-by: Julia Lawall > > --- > > diff -u -p a/drivers/net/pppol2tp.c b/drivers/net/pppol2tp.c > --- linux-2.6/drivers/net/pppol2tp.c 2008-05-09 16:46:57.000000000 +0200 > +++ linuxcopy/drivers/net/pppol2tp.c 2008-05-12 15:30:52.000000000 +0200 > @@ -1621,9 +1621,16 @@ out_no_ppp: > end: > release_sock(sk); > > - if (error != 0) > - PRINTK(session ? session->debug : -1, PPPOL2TP_MSG_CONTROL, KERN_WARNING, > - "%s: connect failed: %d\n", session->name, error); > + if (error != 0) { > + if (session) > + PRINTK(session->debug, > + PPPOL2TP_MSG_CONTROL, KERN_WARNING, > + "%s: connect failed: %d\n", > + session->name, error); > + else > + PRINTK(-1, PPPOL2TP_MSG_CONTROL, KERN_WARNING, > + "connect failed: %d\n", error); > + } > > return error; > } >