* [PATCH] staging: line6: fix possible overrun @ 2014-04-26 17:09 Laurent Navet 2014-04-26 20:47 ` Mateusz Guzik 0 siblings, 1 reply; 12+ messages in thread From: Laurent Navet @ 2014-04-26 17:09 UTC (permalink / raw) To: gregkh; +Cc: tiwai, devel, linux-kernel, 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 <laurent.navet@gmail.com> --- build tested only drivers/staging/line6/audio.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/drivers/staging/line6/audio.c b/drivers/staging/line6/audio.c index 171d80c..65f5cd4 100644 --- a/drivers/staging/line6/audio.c +++ b/drivers/staging/line6/audio.c @@ -32,9 +32,10 @@ int line6_init_audio(struct usb_line6 *line6) line6->card = card; - strcpy(card->id, line6->properties->id); + strncpy(card->id, line6->properties->id, (sizeof(card->id)-1)); strcpy(card->driver, DRIVER_NAME); - strcpy(card->shortname, line6->properties->name); + strncpy(card->shortname, line6->properties->name, + (sizeof(card->shortname)-1)); /* longname is 80 chars - see asound.h */ sprintf(card->longname, "Line6 %s at USB %s", line6->properties->name, dev_name(line6->ifcdev)); -- 1.9.1 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH] staging: line6: fix possible overrun 2014-04-26 17:09 [PATCH] staging: line6: fix possible overrun Laurent Navet @ 2014-04-26 20:47 ` Mateusz Guzik 2014-04-26 21:36 ` Dan Carpenter 0 siblings, 1 reply; 12+ messages in thread From: Mateusz Guzik @ 2014-04-26 20:47 UTC (permalink / raw) To: Laurent Navet; +Cc: gregkh, tiwai, devel, linux-kernel 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 <laurent.navet@gmail.com> > --- > build tested only > > drivers/staging/line6/audio.c | 5 +++-- > 1 file changed, 3 insertions(+), 2 deletions(-) > > diff --git a/drivers/staging/line6/audio.c b/drivers/staging/line6/audio.c > index 171d80c..65f5cd4 100644 > --- a/drivers/staging/line6/audio.c > +++ b/drivers/staging/line6/audio.c > @@ -32,9 +32,10 @@ int line6_init_audio(struct usb_line6 *line6) > > line6->card = card; > > - strcpy(card->id, line6->properties->id); > + strncpy(card->id, line6->properties->id, (sizeof(card->id)-1)); > strcpy(card->driver, DRIVER_NAME); > - strcpy(card->shortname, line6->properties->name); > + strncpy(card->shortname, line6->properties->name, > + (sizeof(card->shortname)-1)); > /* longname is 80 chars - see asound.h */ > sprintf(card->longname, "Line6 %s at USB %s", line6->properties->name, > dev_name(line6->ifcdev)); Would not it be better to return -EINVAL (or some other error) instead? Now you will possibly truncate the name. -- Mateusz Guzik ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] staging: line6: fix possible overrun 2014-04-26 20:47 ` Mateusz Guzik @ 2014-04-26 21:36 ` Dan Carpenter 2014-04-26 21:59 ` Mateusz Guzik 0 siblings, 1 reply; 12+ messages in thread From: Dan Carpenter @ 2014-04-26 21:36 UTC (permalink / raw) To: Mateusz Guzik; +Cc: Laurent Navet, tiwai, gregkh, devel, linux-kernel 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 > > > > Signed-off-by: Laurent Navet <laurent.navet@gmail.com> > > --- > > build tested only > > > > drivers/staging/line6/audio.c | 5 +++-- > > 1 file changed, 3 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/staging/line6/audio.c b/drivers/staging/line6/audio.c > > index 171d80c..65f5cd4 100644 > > --- a/drivers/staging/line6/audio.c > > +++ b/drivers/staging/line6/audio.c > > @@ -32,9 +32,10 @@ int line6_init_audio(struct usb_line6 *line6) > > > > line6->card = card; > > > > - strcpy(card->id, line6->properties->id); > > + strncpy(card->id, line6->properties->id, (sizeof(card->id)-1)); > > strcpy(card->driver, DRIVER_NAME); > > - strcpy(card->shortname, line6->properties->name); > > + strncpy(card->shortname, line6->properties->name, > > + (sizeof(card->shortname)-1)); > > /* longname is 80 chars - see asound.h */ > > sprintf(card->longname, "Line6 %s at USB %s", line6->properties->name, > > dev_name(line6->ifcdev)); > > Would not it be better to return -EINVAL (or some other error) instead? > > Now you will possibly truncate the name. > These don't come from the user, they come from the kernel. drivers/staging/line6/driver.c 59 60 #define L6PROP(dev_bit, dev_id, dev_name, dev_cap)\ 61 {.device_bit = LINE6_BIT_##dev_bit, .id = dev_id,\ 62 .name = dev_name, .capabilities = LINE6_BIT_##dev_cap} 63 64 /* *INDENT-OFF* */ 65 static const struct line6_properties line6_properties_table[] = { 66 L6PROP(BASSPODXT, "BassPODxt", "BassPODxt", CTRL_PCM_HW), 67 L6PROP(BASSPODXTLIVE, "BassPODxtLive", "BassPODxt Live", CTRL_PCM_HW), 68 L6PROP(BASSPODXTPRO, "BassPODxtPro", "BassPODxt Pro", CTRL_PCM_HW), 69 L6PROP(GUITARPORT, "GuitarPort", "GuitarPort", PCM), 70 L6PROP(POCKETPOD, "PocketPOD", "Pocket POD", CONTROL), 71 L6PROP(PODHD300, "PODHD300", "POD HD300", CTRL_PCM_HW), 72 L6PROP(PODHD400, "PODHD400", "POD HD400", CTRL_PCM_HW), 73 L6PROP(PODHD500, "PODHD500", "POD HD500", CTRL_PCM_HW), 74 L6PROP(PODSTUDIO_GX, "PODStudioGX", "POD Studio GX", PCM), 75 L6PROP(PODSTUDIO_UX1, "PODStudioUX1", "POD Studio UX1", PCM), 76 L6PROP(PODSTUDIO_UX2, "PODStudioUX2", "POD Studio UX2", PCM), 77 L6PROP(PODX3, "PODX3", "POD X3", PCM), 78 L6PROP(PODX3LIVE, "PODX3Live", "POD X3 Live", PCM), 79 L6PROP(PODXT, "PODxt", "PODxt", CTRL_PCM_HW), 80 L6PROP(PODXTLIVE, "PODxtLive", "PODxt Live", CTRL_PCM_HW), 81 L6PROP(PODXTPRO, "PODxtPro", "PODxt Pro", CTRL_PCM_HW), 82 L6PROP(TONEPORT_GX, "TonePortGX", "TonePort GX", PCM), 83 L6PROP(TONEPORT_UX1, "TonePortUX1", "TonePort UX1", PCM), 84 L6PROP(TONEPORT_UX2, "TonePortUX2", "TonePort UX2", PCM), 85 L6PROP(VARIAX, "Variax", "Variax Workbench", CONTROL), 86 }; 87 /* *INDENT-ON* */ 88 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 to sovle the truncation issue then we need to think of shorter names for BassPODxtLive, BassPODxtPro, PODStudioUX1, and PODStudioUX2. regards, dan carpenter ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] staging: line6: fix possible overrun 2014-04-26 21:36 ` Dan Carpenter @ 2014-04-26 21:59 ` Mateusz Guzik 2014-04-27 17:39 ` Dan Carpenter 0 siblings, 1 reply; 12+ messages in thread From: Mateusz Guzik @ 2014-04-26 21:59 UTC (permalink / raw) To: Dan Carpenter; +Cc: Laurent Navet, tiwai, gregkh, devel, linux-kernel 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 source buffer is too large. > > > > > > Found by coverity scan : CID 144979 > > > > > > Signed-off-by: Laurent Navet <laurent.navet@gmail.com> > > > --- > > > build tested only > > > > > > drivers/staging/line6/audio.c | 5 +++-- > > > 1 file changed, 3 insertions(+), 2 deletions(-) > > > > > > diff --git a/drivers/staging/line6/audio.c b/drivers/staging/line6/audio.c > > > index 171d80c..65f5cd4 100644 > > > --- a/drivers/staging/line6/audio.c > > > +++ b/drivers/staging/line6/audio.c > > > @@ -32,9 +32,10 @@ int line6_init_audio(struct usb_line6 *line6) > > > > > > line6->card = card; > > > > > > - strcpy(card->id, line6->properties->id); > > > + strncpy(card->id, line6->properties->id, (sizeof(card->id)-1)); > > > strcpy(card->driver, DRIVER_NAME); > > > - strcpy(card->shortname, line6->properties->name); > > > + strncpy(card->shortname, line6->properties->name, > > > + (sizeof(card->shortname)-1)); > > > /* longname is 80 chars - see asound.h */ > > > sprintf(card->longname, "Line6 %s at USB %s", line6->properties->name, > > > dev_name(line6->ifcdev)); > > > > Would not it be better to return -EINVAL (or some other error) instead? > > > > Now you will possibly truncate the name. > > > > These don't come from the user, they come from the kernel. > > drivers/staging/line6/driver.c > 59 > 60 #define L6PROP(dev_bit, dev_id, dev_name, dev_cap)\ > 61 {.device_bit = LINE6_BIT_##dev_bit, .id = dev_id,\ > 62 .name = dev_name, .capabilities = LINE6_BIT_##dev_cap} > 63 > 64 /* *INDENT-OFF* */ > 65 static const struct line6_properties line6_properties_table[] = { > 66 L6PROP(BASSPODXT, "BassPODxt", "BassPODxt", CTRL_PCM_HW), > 67 L6PROP(BASSPODXTLIVE, "BassPODxtLive", "BassPODxt Live", CTRL_PCM_HW), > 68 L6PROP(BASSPODXTPRO, "BassPODxtPro", "BassPODxt Pro", CTRL_PCM_HW), > 69 L6PROP(GUITARPORT, "GuitarPort", "GuitarPort", PCM), > 70 L6PROP(POCKETPOD, "PocketPOD", "Pocket POD", CONTROL), > 71 L6PROP(PODHD300, "PODHD300", "POD HD300", CTRL_PCM_HW), > 72 L6PROP(PODHD400, "PODHD400", "POD HD400", CTRL_PCM_HW), > 73 L6PROP(PODHD500, "PODHD500", "POD HD500", CTRL_PCM_HW), > 74 L6PROP(PODSTUDIO_GX, "PODStudioGX", "POD Studio GX", PCM), > 75 L6PROP(PODSTUDIO_UX1, "PODStudioUX1", "POD Studio UX1", PCM), > 76 L6PROP(PODSTUDIO_UX2, "PODStudioUX2", "POD Studio UX2", PCM), > 77 L6PROP(PODX3, "PODX3", "POD X3", PCM), > 78 L6PROP(PODX3LIVE, "PODX3Live", "POD X3 Live", PCM), > 79 L6PROP(PODXT, "PODxt", "PODxt", CTRL_PCM_HW), > 80 L6PROP(PODXTLIVE, "PODxtLive", "PODxt Live", CTRL_PCM_HW), > 81 L6PROP(PODXTPRO, "PODxtPro", "PODxt Pro", CTRL_PCM_HW), > 82 L6PROP(TONEPORT_GX, "TonePortGX", "TonePort GX", PCM), > 83 L6PROP(TONEPORT_UX1, "TonePortUX1", "TonePort UX1", PCM), > 84 L6PROP(TONEPORT_UX2, "TonePortUX2", "TonePort UX2", PCM), > 85 L6PROP(VARIAX, "Variax", "Variax Workbench", CONTROL), > 86 }; > 87 /* *INDENT-ON* */ > 88 > > 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 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 and a WARN_ON + -EINVAL in line6_init_audio to catch future offenders. As a side note I'm not sure if pod_try_init from drivers/staging/line6/pod.c cleans up properly after failed line6_init_audio. -- Mateusz Guzik ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] staging: line6: fix possible overrun 2014-04-26 21:59 ` Mateusz Guzik @ 2014-04-27 17:39 ` Dan Carpenter 2014-04-27 19:05 ` Laurent Navet 2014-04-27 20:00 ` Mateusz Guzik 0 siblings, 2 replies; 12+ messages in thread From: Dan Carpenter @ 2014-04-27 17:39 UTC (permalink / raw) To: Mateusz Guzik; +Cc: tiwai, gregkh, devel, Laurent Navet, linux-kernel 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 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 That sounds like some magic code which I would love to see. :) > 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. > > As a side note I'm not sure if pod_try_init from drivers/staging/line6/pod.c > cleans up properly after failed line6_init_audio. Yeah. It doesn't seem to clean up at all. regards, dan carpenter ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] staging: line6: fix possible overrun 2014-04-27 17:39 ` Dan Carpenter @ 2014-04-27 19:05 ` Laurent Navet 2014-04-27 20:00 ` Mateusz Guzik 1 sibling, 0 replies; 12+ messages in thread From: Laurent Navet @ 2014-04-27 19:05 UTC (permalink / raw) To: Dan Carpenter; +Cc: Mateusz Guzik, tiwai, gregkh, devel, linux-kernel 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 > > That sounds like some magic code which I would love to see. :) > >> 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. > >> >> As a side note I'm not sure if pod_try_init from >> drivers/staging/line6/pod.c >> cleans up properly after failed line6_init_audio. > > Yeah. It doesn't seem to clean up at all. > Laurent Navet. -- « On ne résout pas un problème avec les modes de pensée qui l’ont engendré. » « You cannot solve current problems with current thinking. Current problems are the result of current thinking » ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] staging: line6: fix possible overrun 2014-04-27 17:39 ` Dan Carpenter 2014-04-27 19:05 ` Laurent Navet @ 2014-04-27 20:00 ` Mateusz Guzik 2014-04-27 22:44 ` Dan Carpenter 1 sibling, 1 reply; 12+ messages in thread From: Mateusz Guzik @ 2014-04-27 20:00 UTC (permalink / raw) To: Dan Carpenter; +Cc: tiwai, gregkh, devel, Laurent Navet, linux-kernel 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->shortname is a 32 char array so none of those overflow. > > > > > > 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 > > That sounds like some magic code which I would love to see. :) > Just asserting something on compile time is not a problem. The kernel has BUILD_BUG_ON macro. I didn't check why, but it doesn't use _Static_assert. Instead it produces some code which makes it unusable in this context. Aforementoined _Static_assert is available at least in gcc and clang and you can call it outside of any function, e.g.: _Static_assert(sizeof(meh) < 42, "oh no"); Unfortnately I failed to come up with a macro which would allow me to use it in the initializer. :/ One could change line6_properties's definition so that it contains arrays instead of pointers, that would introduce automagic checking and I don't think memory waste (if any) would be problematic. > > 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 returning the error anyway. Something is wrong, better fix it as it is instead of risking additional bugs resulting from truncation. > > > > As a side note I'm not sure if pod_try_init from drivers/staging/line6/pod.c > > cleans up properly after failed line6_init_audio. > > Yeah. It doesn't seem to clean up at all. > -- Mateusz Guzik ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] staging: line6: fix possible overrun 2014-04-27 20:00 ` Mateusz Guzik @ 2014-04-27 22:44 ` Dan Carpenter 2014-04-29 14:47 ` Takashi Iwai 0 siblings, 1 reply; 12+ messages in thread From: Dan Carpenter @ 2014-04-27 22:44 UTC (permalink / raw) To: Mateusz Guzik; +Cc: tiwai, gregkh, devel, Laurent Navet, linux-kernel 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 returning the error anyway. I'm trying to be polite, but you are talking about adding regressions deliberately... It's very rare for people to deliberately add regressions to the kernel. I have only seen it one time before. regards, dan carpenter ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] staging: line6: fix possible overrun 2014-04-27 22:44 ` Dan Carpenter @ 2014-04-29 14:47 ` Takashi Iwai 2014-04-29 15:02 ` Dan Carpenter 2014-04-29 15:26 ` Mateusz Guzik 0 siblings, 2 replies; 12+ messages in thread From: Takashi Iwai @ 2014-04-29 14:47 UTC (permalink / raw) To: Dan Carpenter; +Cc: Mateusz Guzik, gregkh, devel, Laurent Navet, linux-kernel 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 > > > completely and make it unusable. > > > > > > > Well I would vote for returning the error anyway. > > I'm trying to be polite, but you are talking about adding regressions > deliberately... > > It's very rare for people to deliberately add regressions to the kernel. > I have only seen it one time before. I don't think Dan would be against returning -EINVAL if all the offender codes have been fixed (e.g. truncating strings to fit with the fixed arrays) at first. Then it'd be a good help to catch any future bugs. But, having -EINVAL without fixing the caller side means essentially that you're introducing the breakage intentionally although you know it certainly breaks, which is obviously bad. Takashi ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] staging: line6: fix possible overrun 2014-04-29 14:47 ` Takashi Iwai @ 2014-04-29 15:02 ` Dan Carpenter 2014-04-29 15:26 ` Mateusz Guzik 1 sibling, 0 replies; 12+ messages in thread From: Dan Carpenter @ 2014-04-29 15:02 UTC (permalink / raw) To: Takashi Iwai; +Cc: Mateusz Guzik, gregkh, devel, Laurent Navet, linux-kernel 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 months I will have improved the Smatch code to know that the source string is too large so this bug could have been avoided. regards, dan carpenter ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] staging: line6: fix possible overrun 2014-04-29 14:47 ` Takashi Iwai 2014-04-29 15:02 ` Dan Carpenter @ 2014-04-29 15:26 ` Mateusz Guzik 2014-04-29 17:28 ` Dan Carpenter 1 sibling, 1 reply; 12+ messages in thread From: Mateusz Guzik @ 2014-04-29 15:26 UTC (permalink / raw) To: Takashi Iwai; +Cc: Dan Carpenter, gregkh, devel, Laurent Navet, linux-kernel 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. > > > > > > > > Returning -EINVAL is a bad idea because it would break the driver > > > > completely and make it unusable. > > > > > > > > > > Well I would vote for returning the error anyway. > > > > I'm trying to be polite, but you are talking about adding regressions > > deliberately... > > > > It's very rare for people to deliberately add regressions to the kernel. > > I have only seen it one time before. > > I don't think Dan would be against returning -EINVAL if all the > offender codes have been fixed (e.g. truncating strings to fit with > the fixed arrays) at first. Then it'd be a good help to catch any > future bugs. But, having -EINVAL without fixing the caller side means > essentially that you're introducing the breakage intentionally > although you know it certainly breaks, which is obviously bad. > > We clearly have a serious miscommunication here (and apparently it started with me not addressing the concern of complete driver breakage). line6_init_audio consumers have to be fixed first, no doubt about that. I was only commenting on catching *future* offenders, which I thought would implictly mean *afterwards*. With that in mind it would seem we are in agreement after all. :-) As far getting this done maybe OP is interested. -- Mateusz Guzik ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] staging: line6: fix possible overrun 2014-04-29 15:26 ` Mateusz Guzik @ 2014-04-29 17:28 ` Dan Carpenter 0 siblings, 0 replies; 12+ messages in thread From: Dan Carpenter @ 2014-04-29 17:28 UTC (permalink / raw) To: Mateusz Guzik; +Cc: Takashi Iwai, devel, gregkh, Laurent Navet, linux-kernel 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. It's also unlikely that we will introduce any new truncation bugs. Breaking the driver is very serious. But on the other hand as soon as you proposed introducing an -EINVAL, I reviewed the driver and found the places which would break when the code was changed. As a result of my review then the likely hood of breakage is now low. In other words both options are probably fine. If someone is able to test the -EINVAL change then I would be happy to have it. regards, dan carpenter ^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2014-04-29 17:29 UTC | newest] Thread overview: 12+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2014-04-26 17:09 [PATCH] staging: line6: fix possible overrun Laurent Navet 2014-04-26 20:47 ` Mateusz Guzik 2014-04-26 21:36 ` Dan Carpenter 2014-04-26 21:59 ` Mateusz Guzik 2014-04-27 17:39 ` Dan Carpenter 2014-04-27 19:05 ` Laurent Navet 2014-04-27 20:00 ` Mateusz Guzik 2014-04-27 22:44 ` Dan Carpenter 2014-04-29 14:47 ` Takashi Iwai 2014-04-29 15:02 ` Dan Carpenter 2014-04-29 15:26 ` Mateusz Guzik 2014-04-29 17:28 ` Dan Carpenter
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox