Message ID | 1300997220-4354-1-git-send-email-jarod@redhat.com (mailing list archive) |
---|---|
State | Superseded, archived |
Headers |
Return-path: <mchehab@pedra> Envelope-to: mchehab@pedra Delivery-date: Thu, 24 Mar 2011 17:08:14 -0300 Received: from mchehab by pedra with local (Exim 4.72) (envelope-from <mchehab@pedra>) id 1Q2qpK-0007Oa-DA for mchehab@pedra; Thu, 24 Mar 2011 17:08:14 -0300 Received: from casper.infradead.org [85.118.1.10] by pedra with IMAP (fetchmail-6.3.17) for <mchehab@localhost> (single-drop); Thu, 24 Mar 2011 17:08:14 -0300 (BRT) Received: from vger.kernel.org ([209.132.180.67]) by casper.infradead.org with esmtp (Exim 4.72 #1 (Red Hat Linux)) id 1Q2qoI-0004fq-PK; Thu, 24 Mar 2011 20:07:11 +0000 Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755227Ab1CXUHI (ORCPT <rfc822; kmpark@infradead.org> + 1 other); Thu, 24 Mar 2011 16:07:08 -0400 Received: from mx1.redhat.com ([209.132.183.28]:15824 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752716Ab1CXUHG (ORCPT <rfc822;linux-media@vger.kernel.org>); Thu, 24 Mar 2011 16:07:06 -0400 Received: from int-mx12.intmail.prod.int.phx2.redhat.com (int-mx12.intmail.prod.int.phx2.redhat.com [10.5.11.25]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id p2OK74of022741 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK); Thu, 24 Mar 2011 16:07:04 -0400 Received: from xavier.bos.redhat.com (xavier.bos.redhat.com [10.16.16.50]) by int-mx12.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id p2OK73Pr032463; Thu, 24 Mar 2011 16:07:04 -0400 From: Jarod Wilson <jarod@redhat.com> To: linux-media@vger.kernel.org Cc: Jarod Wilson <jarod@redhat.com>, devel@driverdev.osuosl.org Subject: [PATCH] tm6000: fix vbuf may be used uninitialized Date: Thu, 24 Mar 2011 16:07:00 -0400 Message-Id: <1300997220-4354-1-git-send-email-jarod@redhat.com> X-Scanned-By: MIMEDefang 2.68 on 10.5.11.25 Precedence: bulk List-ID: <linux-media.vger.kernel.org> X-Mailing-List: linux-media@vger.kernel.org Sender: <mchehab@pedra> |
Commit Message
Jarod Wilson
March 24, 2011, 8:07 p.m. UTC
Signed-off-by: Jarod Wilson <jarod@redhat.com>
CC: devel@driverdev.osuosl.org
---
drivers/staging/tm6000/tm6000-video.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
Comments
On Thu, Mar 24, 2011 at 04:07:00PM -0400, Jarod Wilson wrote: > Signed-off-by: Jarod Wilson <jarod@redhat.com> Jarod, there is a lot of information missing from your change log... :/ > CC: devel@driverdev.osuosl.org > --- > drivers/staging/tm6000/tm6000-video.c | 2 +- > 1 files changed, 1 insertions(+), 1 deletions(-) > > diff --git a/drivers/staging/tm6000/tm6000-video.c b/drivers/staging/tm6000/tm6000-video.c > index c80a316..bfebedd 100644 > --- a/drivers/staging/tm6000/tm6000-video.c > +++ b/drivers/staging/tm6000/tm6000-video.c > @@ -228,7 +228,7 @@ static int copy_streams(u8 *data, unsigned long len, > unsigned long header = 0; > int rc = 0; > unsigned int cmd, cpysize, pktsize, size, field, block, line, pos = 0; > - struct tm6000_buffer *vbuf; > + struct tm6000_buffer *vbuf = NULL; > char *voutp = NULL; > unsigned int linewidth; > This looks like a real bug versus just a GCC warning. It was introduced in 8aff8ba95155df "[media] tm6000: add radio support to the driver". I've added Dmitri to the CC list. regards, dan carpenter -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, Mar 24, 2011 at 11:31:08PM +0300, Dan Carpenter wrote: > On Thu, Mar 24, 2011 at 04:07:00PM -0400, Jarod Wilson wrote: > > Signed-off-by: Jarod Wilson <jarod@redhat.com> > > Jarod, there is a lot of information missing from your change log... :/ Hrm, I'm building the media stack with all warnings fatal, so this was just a quick fix to silence the compiler warning, didn't really look into it at all. > > CC: devel@driverdev.osuosl.org > > --- > > drivers/staging/tm6000/tm6000-video.c | 2 +- > > 1 files changed, 1 insertions(+), 1 deletions(-) > > > > diff --git a/drivers/staging/tm6000/tm6000-video.c b/drivers/staging/tm6000/tm6000-video.c > > index c80a316..bfebedd 100644 > > --- a/drivers/staging/tm6000/tm6000-video.c > > +++ b/drivers/staging/tm6000/tm6000-video.c > > @@ -228,7 +228,7 @@ static int copy_streams(u8 *data, unsigned long len, > > unsigned long header = 0; > > int rc = 0; > > unsigned int cmd, cpysize, pktsize, size, field, block, line, pos = 0; > > - struct tm6000_buffer *vbuf; > > + struct tm6000_buffer *vbuf = NULL; > > char *voutp = NULL; > > unsigned int linewidth; > > > > This looks like a real bug versus just a GCC warning. It was introduced > in 8aff8ba95155df "[media] tm6000: add radio support to the driver". > I've added Dmitri to the CC list. Thanks much, will try to pay more attention next time. ;)
On Mar 24, 2011, at 6:05 PM, Jarod Wilson wrote: > On Thu, Mar 24, 2011 at 11:31:08PM +0300, Dan Carpenter wrote: >> On Thu, Mar 24, 2011 at 04:07:00PM -0400, Jarod Wilson wrote: >>> Signed-off-by: Jarod Wilson <jarod@redhat.com> >> >> Jarod, there is a lot of information missing from your change log... :/ > > Hrm, I'm building the media stack with all warnings fatal, so this was > just a quick fix to silence the compiler warning, didn't really look into > it at all. > >>> CC: devel@driverdev.osuosl.org >>> --- >>> drivers/staging/tm6000/tm6000-video.c | 2 +- >>> 1 files changed, 1 insertions(+), 1 deletions(-) >>> >>> diff --git a/drivers/staging/tm6000/tm6000-video.c b/drivers/staging/tm6000/tm6000-video.c >>> index c80a316..bfebedd 100644 >>> --- a/drivers/staging/tm6000/tm6000-video.c >>> +++ b/drivers/staging/tm6000/tm6000-video.c >>> @@ -228,7 +228,7 @@ static int copy_streams(u8 *data, unsigned long len, >>> unsigned long header = 0; >>> int rc = 0; >>> unsigned int cmd, cpysize, pktsize, size, field, block, line, pos = 0; >>> - struct tm6000_buffer *vbuf; >>> + struct tm6000_buffer *vbuf = NULL; >>> char *voutp = NULL; >>> unsigned int linewidth; >>> >> >> This looks like a real bug versus just a GCC warning. It was introduced >> in 8aff8ba95155df "[media] tm6000: add radio support to the driver". >> I've added Dmitri to the CC list. > > Thanks much, will try to pay more attention next time. ;) So I was just circling back around on this one, and took some time to read the actual code and the radio support addition. After doing so, I don't see why the patch I proposed wouldn't do. The buffer is only manipulated if !dev->radio or if vbuf is non-NULL (the memcpy call). If its initialized to NULL, it only gets used exactly as it did before 8aff8ba9 when !dev->radio, and if its not been used or its NULL following manipulations protected by !dev->radio, it doesn't get copied. What is the "real bug" I am missing there? (Or did I already miss a patch posted to linux-media addressing it?) Thanks much,
diff --git a/drivers/staging/tm6000/tm6000-video.c b/drivers/staging/tm6000/tm6000-video.c index c80a316..bfebedd 100644 --- a/drivers/staging/tm6000/tm6000-video.c +++ b/drivers/staging/tm6000/tm6000-video.c @@ -228,7 +228,7 @@ static int copy_streams(u8 *data, unsigned long len, unsigned long header = 0; int rc = 0; unsigned int cmd, cpysize, pktsize, size, field, block, line, pos = 0; - struct tm6000_buffer *vbuf; + struct tm6000_buffer *vbuf = NULL; char *voutp = NULL; unsigned int linewidth;