* [PATCH] tm6000: fix vbuf may be used uninitialized @ 2011-03-24 20:07 Jarod Wilson 2011-03-24 20:31 ` Dan Carpenter 2011-04-12 18:48 ` [PATCH v2] " Jarod Wilson 0 siblings, 2 replies; 6+ messages in thread From: Jarod Wilson @ 2011-03-24 20:07 UTC (permalink / raw) To: linux-media; +Cc: Jarod Wilson, devel 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(-) 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; -- 1.7.1 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] tm6000: fix vbuf may be used uninitialized 2011-03-24 20:07 [PATCH] tm6000: fix vbuf may be used uninitialized Jarod Wilson @ 2011-03-24 20:31 ` Dan Carpenter 2011-03-24 22:05 ` Jarod Wilson 2011-04-12 18:48 ` [PATCH v2] " Jarod Wilson 1 sibling, 1 reply; 6+ messages in thread From: Dan Carpenter @ 2011-03-24 20:31 UTC (permalink / raw) To: Jarod Wilson, Dmitri Belimov; +Cc: linux-media, devel 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 ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] tm6000: fix vbuf may be used uninitialized 2011-03-24 20:31 ` Dan Carpenter @ 2011-03-24 22:05 ` Jarod Wilson 2011-04-11 21:48 ` Jarod Wilson 0 siblings, 1 reply; 6+ messages in thread From: Jarod Wilson @ 2011-03-24 22:05 UTC (permalink / raw) To: Dan Carpenter; +Cc: Dmitri Belimov, linux-media, devel 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. ;) -- Jarod Wilson jarod@redhat.com ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] tm6000: fix vbuf may be used uninitialized 2011-03-24 22:05 ` Jarod Wilson @ 2011-04-11 21:48 ` Jarod Wilson 0 siblings, 0 replies; 6+ messages in thread From: Jarod Wilson @ 2011-04-11 21:48 UTC (permalink / raw) To: Jarod Wilson; +Cc: Dan Carpenter, Dmitri Belimov, linux-media, devel 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, -- Jarod Wilson jarod@wilsonet.com ^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH v2] tm6000: fix vbuf may be used uninitialized 2011-03-24 20:07 [PATCH] tm6000: fix vbuf may be used uninitialized Jarod Wilson 2011-03-24 20:31 ` Dan Carpenter @ 2011-04-12 18:48 ` Jarod Wilson 2011-04-15 2:04 ` Dmitri Belimov 1 sibling, 1 reply; 6+ messages in thread From: Jarod Wilson @ 2011-04-12 18:48 UTC (permalink / raw) To: linux-media; +Cc: Jarod Wilson, Dan Carpenter, Dmitri Belimov, devel In commit 8aff8ba95155df, most of the manipulations to vbuf inside copy_streams were gated on if !dev->radio, but one place that touches vbuf lays outside those gates -- a memcpy of vbuf isn't NULL. If we initialize vbuf to NULL, that memcpy will never happen in the case where we do have dev->radio, and otherwise, in the !dev->radio case, the code behaves exactly like it did prior to 8aff8ba95155df. While we're at it, also fix an incorrectly indented closing brace for one of the sections touching vbuf that is conditional on !dev->radio. v2: add a detailed commit log and fix that brace CC: Dan Carpenter <error27@gmail.com> CC: Dmitri Belimov <d.belimov@gmail.com> CC: devel@driverdev.osuosl.org Signed-off-by: Jarod Wilson <jarod@redhat.com> --- drivers/staging/tm6000/tm6000-video.c | 4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/staging/tm6000/tm6000-video.c b/drivers/staging/tm6000/tm6000-video.c index c80a316..8b971a0 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; @@ -318,7 +318,7 @@ static int copy_streams(u8 *data, unsigned long len, if (pos + size > vbuf->vb.size) cmd = TM6000_URB_MSG_ERR; dev->isoc_ctl.vfield = field; - } + } break; case TM6000_URB_MSG_VBI: break; -- 1.7.1 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH v2] tm6000: fix vbuf may be used uninitialized 2011-04-12 18:48 ` [PATCH v2] " Jarod Wilson @ 2011-04-15 2:04 ` Dmitri Belimov 0 siblings, 0 replies; 6+ messages in thread From: Dmitri Belimov @ 2011-04-15 2:04 UTC (permalink / raw) To: Jarod Wilson; +Cc: linux-media, Dan Carpenter, devel Hi I think it's good. No regression, all works well. With my best regards, Dmitry. > In commit 8aff8ba95155df, most of the manipulations to vbuf inside > copy_streams were gated on if !dev->radio, but one place that touches > vbuf lays outside those gates -- a memcpy of vbuf isn't NULL. If we > initialize vbuf to NULL, that memcpy will never happen in the case > where we do have dev->radio, and otherwise, in the !dev->radio case, > the code behaves exactly like it did prior to 8aff8ba95155df. > > While we're at it, also fix an incorrectly indented closing brace for > one of the sections touching vbuf that is conditional on !dev->radio. > > v2: add a detailed commit log and fix that brace > > CC: Dan Carpenter <error27@gmail.com> > CC: Dmitri Belimov <d.belimov@gmail.com> > CC: devel@driverdev.osuosl.org > Signed-off-by: Jarod Wilson <jarod@redhat.com> > --- > drivers/staging/tm6000/tm6000-video.c | 4 ++-- > 1 files changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/staging/tm6000/tm6000-video.c > b/drivers/staging/tm6000/tm6000-video.c index c80a316..8b971a0 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; > > @@ -318,7 +318,7 @@ static int copy_streams(u8 *data, unsigned long > len, if (pos + size > vbuf->vb.size) > cmd = > TM6000_URB_MSG_ERR; dev->isoc_ctl.vfield = field; > - } > + } > break; > case TM6000_URB_MSG_VBI: > break; > -- > 1.7.1 > ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2011-04-15 3:01 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2011-03-24 20:07 [PATCH] tm6000: fix vbuf may be used uninitialized Jarod Wilson 2011-03-24 20:31 ` Dan Carpenter 2011-03-24 22:05 ` Jarod Wilson 2011-04-11 21:48 ` Jarod Wilson 2011-04-12 18:48 ` [PATCH v2] " Jarod Wilson 2011-04-15 2:04 ` Dmitri Belimov
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox