Re: [PATCH] staging: line6: fix possible overrun

2014-04-29 Thread Dan Carpenter
We have two options: 1) Do we risk introducing new truncation bugs. 2) Do we risk breaking the driver because we didn't catch every truncation bug. Truncation bugs here are very low impact and probably no one would even notice. That's how not worried I am about truncation bugs in this context.

Re: [PATCH] staging: line6: fix possible overrun

2014-04-29 Thread Mateusz Guzik
On Tue, Apr 29, 2014 at 04:47:11PM +0200, Takashi Iwai wrote: > At Mon, 28 Apr 2014 01:44:25 +0300, > Dan Carpenter wrote: > > > > On Sun, Apr 27, 2014 at 10:00:43PM +0200, Mateusz Guzik wrote: > > > > > and a WARN_ON + -EINVAL in line6_init_audio to catch future > > > > > offenders. > > > > >

Re: [PATCH] staging: line6: fix possible overrun

2014-04-29 Thread Dan Carpenter
Yeah. If this were a brand new driver then returning -EINVAL would be a good idea. Smatch actually warns about this code as well if you turn on the --spammy option. But there are too many of these kinds of warnings and even I can't check them all so the warning is basically useless. In a few mo

Re: [PATCH] staging: line6: fix possible overrun

2014-04-29 Thread Takashi Iwai
At Mon, 28 Apr 2014 01:44:25 +0300, Dan Carpenter wrote: > > On Sun, Apr 27, 2014 at 10:00:43PM +0200, Mateusz Guzik wrote: > > > > and a WARN_ON + -EINVAL in line6_init_audio to catch future > > > > offenders. > > > > > > Returning -EINVAL is a bad idea because it would break the driver > > >

Re: [PATCH] staging: line6: fix possible overrun

2014-04-27 Thread Dan Carpenter
On Sun, Apr 27, 2014 at 10:00:43PM +0200, Mateusz Guzik wrote: > > > and a WARN_ON + -EINVAL in line6_init_audio to catch future > > > offenders. > > > > Returning -EINVAL is a bad idea because it would break the driver > > completely and make it unusable. > > > > Well I would vote for returni

Re: [PATCH] staging: line6: fix possible overrun

2014-04-27 Thread Mateusz Guzik
On Sun, Apr 27, 2014 at 08:39:32PM +0300, Dan Carpenter wrote: > On Sat, Apr 26, 2014 at 11:59:46PM +0200, Mateusz Guzik wrote: > > > And sadly enough some of those ->id strings are more than 15 characters > > > and a NUL which will fit in card->id. So this overflow is real. The > > > card->short

Re: [PATCH] staging: line6: fix possible overrun

2014-04-27 Thread Laurent Navet
Thank's for your answers, will try to look deeper, >> > >> > If we want to sovle the truncation issue then we need to think of >> > shorter names for BassPODxtLive, BassPODxtPro, PODStudioUX1, and >> > PODStudioUX2. >> > >> >> In that case I suggest compile time assertions that ids and names fit >

Re: [PATCH] staging: line6: fix possible overrun

2014-04-27 Thread Dan Carpenter
On Sat, Apr 26, 2014 at 11:59:46PM +0200, Mateusz Guzik wrote: > > And sadly enough some of those ->id strings are more than 15 characters > > and a NUL which will fit in card->id. So this overflow is real. The > > card->shortname is a 32 char array so none of those overflow. > > > > If we want

Re: [PATCH] staging: line6: fix possible overrun

2014-04-26 Thread Mateusz Guzik
On Sun, Apr 27, 2014 at 12:36:21AM +0300, Dan Carpenter wrote: > On Sat, Apr 26, 2014 at 10:47:05PM +0200, Mateusz Guzik wrote: > > On Sat, Apr 26, 2014 at 07:09:22PM +0200, Laurent Navet wrote: > > > The strcpy operation may write past the end of the fixed-size destination > > > buffer if the sour

Re: [PATCH] staging: line6: fix possible overrun

2014-04-26 Thread Dan Carpenter
On Sat, Apr 26, 2014 at 10:47:05PM +0200, Mateusz Guzik wrote: > On Sat, Apr 26, 2014 at 07:09:22PM +0200, Laurent Navet wrote: > > The strcpy operation may write past the end of the fixed-size destination > > buffer if the source buffer is too large. > > > > Found by coverity scan : CID 144979 >

Re: [PATCH] staging: line6: fix possible overrun

2014-04-26 Thread Mateusz Guzik
On Sat, Apr 26, 2014 at 07:09:22PM +0200, Laurent Navet wrote: > The strcpy operation may write past the end of the fixed-size destination > buffer if the source buffer is too large. > > Found by coverity scan : CID 144979 > > Signed-off-by: Laurent Navet > --- > build tested only > > drivers/

[PATCH] staging: line6: fix possible overrun

2014-04-26 Thread Laurent Navet
The strcpy operation may write past the end of the fixed-size destination buffer if the source buffer is too large. Found by coverity scan : CID 144979 Signed-off-by: Laurent Navet --- build tested only drivers/staging/line6/audio.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) d