Re: [PATCH] staging: line6: toneport.c: Fix for possible null pointer dereference

[Stefan Hajnoczi]

On Sun, Dec 21, 2014 at 10:43 PM, Rickard Strandqvist
 wrote:
> The NULL check was done to late, and there it was a risk
> of a possible null pointer dereference.
>
> This was partially found by using a static code analysis program called 
> cppcheck.
>
> Signed-off-by: Rickard Strandqvist 
> ---
>  drivers/staging/line6/toneport.c |   15 ---
>  1 file changed, 8 insertions(+), 7 deletions(-)

Reviewed-by: Stefan Hajnoczi 
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH] staging: line6: toneport.c: Fix for possible null pointer dereference

[Rickard Strandqvist]

The NULL check was done to late, and there it was a risk
of a possible null pointer dereference.

This was partially found by using a static code analysis program called 
cppcheck.

Signed-off-by: Rickard Strandqvist 
---
 drivers/staging/line6/toneport.c |   15 ---
 1 file changed, 8 insertions(+), 7 deletions(-)

diff --git a/drivers/staging/line6/toneport.c b/drivers/staging/line6/toneport.c
index 6943715..660dc3f 100644
--- a/drivers/staging/line6/toneport.c
+++ b/drivers/staging/line6/toneport.c
@@ -433,12 +433,16 @@ void line6_toneport_reset_resume(struct 
usb_line6_toneport *toneport)
 void line6_toneport_disconnect(struct usb_interface *interface)
 {
struct usb_line6_toneport *toneport;
+   struct snd_line6_pcm *line6pcm;
u16 idProduct;
 
if (interface == NULL)
return;
 
toneport = usb_get_intfdata(interface);
+   if (NULL == toneport)
+   return;
+
del_timer_sync(>timer);
idProduct = le16_to_cpu(toneport->line6.usbdev->descriptor.idProduct);
 
@@ -447,13 +451,10 @@ void line6_toneport_disconnect(struct usb_interface 
*interface)
device_remove_file(>dev, _attr_led_green);
}
 
-   if (toneport != NULL) {
-   struct snd_line6_pcm *line6pcm = toneport->line6.line6pcm;
-
-   if (line6pcm != NULL) {
-   line6_pcm_release(line6pcm, LINE6_BITS_PCM_MONITOR);
-   line6_pcm_disconnect(line6pcm);
-   }
+   line6pcm = toneport->line6.line6pcm;
+   if (line6pcm != NULL) {
+   line6_pcm_release(line6pcm, LINE6_BITS_PCM_MONITOR);
+   line6_pcm_disconnect(line6pcm);
}
 
toneport_destruct(interface);
-- 
1.7.10.4

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH] staging: line6: toneport.c: Fix for possible null pointer dereference

[Rickard Strandqvist]

The NULL check was done to late, and there it was a risk
of a possible null pointer dereference.

This was partially found by using a static code analysis program called 
cppcheck.

Signed-off-by: Rickard Strandqvist rickard_strandqv...@spectrumdigital.se
---
 drivers/staging/line6/toneport.c |   15 ---
 1 file changed, 8 insertions(+), 7 deletions(-)

diff --git a/drivers/staging/line6/toneport.c b/drivers/staging/line6/toneport.c
index 6943715..660dc3f 100644
--- a/drivers/staging/line6/toneport.c
+++ b/drivers/staging/line6/toneport.c
@@ -433,12 +433,16 @@ void line6_toneport_reset_resume(struct 
usb_line6_toneport *toneport)
 void line6_toneport_disconnect(struct usb_interface *interface)
 {
struct usb_line6_toneport *toneport;
+   struct snd_line6_pcm *line6pcm;
u16 idProduct;
 
if (interface == NULL)
return;
 
toneport = usb_get_intfdata(interface);
+   if (NULL == toneport)
+   return;
+
del_timer_sync(toneport-timer);
idProduct = le16_to_cpu(toneport-line6.usbdev-descriptor.idProduct);
 
@@ -447,13 +451,10 @@ void line6_toneport_disconnect(struct usb_interface 
*interface)
device_remove_file(interface-dev, dev_attr_led_green);
}
 
-   if (toneport != NULL) {
-   struct snd_line6_pcm *line6pcm = toneport-line6.line6pcm;
-
-   if (line6pcm != NULL) {
-   line6_pcm_release(line6pcm, LINE6_BITS_PCM_MONITOR);
-   line6_pcm_disconnect(line6pcm);
-   }
+   line6pcm = toneport-line6.line6pcm;
+   if (line6pcm != NULL) {
+   line6_pcm_release(line6pcm, LINE6_BITS_PCM_MONITOR);
+   line6_pcm_disconnect(line6pcm);
}
 
toneport_destruct(interface);
-- 
1.7.10.4

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] staging: line6: toneport.c: Fix for possible null pointer dereference

[Stefan Hajnoczi]

On Sun, Dec 21, 2014 at 10:43 PM, Rickard Strandqvist
rickard_strandqv...@spectrumdigital.se wrote:
 The NULL check was done to late, and there it was a risk
 of a possible null pointer dereference.

 This was partially found by using a static code analysis program called 
 cppcheck.

 Signed-off-by: Rickard Strandqvist rickard_strandqv...@spectrumdigital.se
 ---
  drivers/staging/line6/toneport.c |   15 ---
  1 file changed, 8 insertions(+), 7 deletions(-)

Reviewed-by: Stefan Hajnoczi stefa...@gmail.com
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] staging: line6: toneport.c: Fix for possible null pointer dereference

[Dan Carpenter]

On Mon, May 19, 2014 at 11:39:00PM +0200, Rickard Strandqvist wrote:
> There is otherwise a risk of a possible null pointer dereference.
> 
> Was largely found by using a static code analysis program called cppcheck.
> 
> Signed-off-by: Rickard Strandqvist 

This one is called from line6_disconnect() and the caller checks that
usb_get_intfdata(interface); returns non-NULL.  So it's messy code but
not a real bug.

For the lustre ones, I don't know if they are real bugs or not.

regards,
dan carpenter

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] staging: line6: toneport.c: Fix for possible null pointer dereference

[Stefan Hajnoczi]

On Mon, May 19, 2014 at 11:39 PM, Rickard Strandqvist
 wrote:
> There is otherwise a risk of a possible null pointer dereference.
>
> Was largely found by using a static code analysis program called cppcheck.
>
> Signed-off-by: Rickard Strandqvist 
> ---
>  drivers/staging/line6/toneport.c |   14 --
>  1 file changed, 8 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/staging/line6/toneport.c 
> b/drivers/staging/line6/toneport.c
> index af2e7e5..36fe76d 100644
> --- a/drivers/staging/line6/toneport.c
> +++ b/drivers/staging/line6/toneport.c
> @@ -431,11 +431,15 @@ void line6_toneport_disconnect(struct usb_interface 
> *interface)
>  {
> struct usb_line6_toneport *toneport;
> u16 idProduct;
> +   struct snd_line6_pcm *line6pcm;
>
> if (interface == NULL)
> return;
>
> toneport = usb_get_intfdata(interface);
> +   if (toneport == NULL)
> +   return;
> +
> del_timer_sync(>timer);
> idProduct = le16_to_cpu(toneport->line6.usbdev->descriptor.idProduct);
>
> @@ -444,13 +448,11 @@ void line6_toneport_disconnect(struct usb_interface 
> *interface)
> device_remove_file(>dev, _attr_led_green);
> }
>
> -   if (toneport != NULL) {
> -   struct snd_line6_pcm *line6pcm = toneport->line6.line6pcm;

Didn't look into this but it's the traditional "use before NULL
check".  Can't hurt to fix it.

Reviewed-by: Stefan Hajnoczi 
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] staging: line6: toneport.c: Fix for possible null pointer dereference

[Stefan Hajnoczi]

On Mon, May 19, 2014 at 11:39 PM, Rickard Strandqvist
rickard_strandqv...@spectrumdigital.se wrote:
 There is otherwise a risk of a possible null pointer dereference.

 Was largely found by using a static code analysis program called cppcheck.

 Signed-off-by: Rickard Strandqvist rickard_strandqv...@spectrumdigital.se
 ---
  drivers/staging/line6/toneport.c |   14 --
  1 file changed, 8 insertions(+), 6 deletions(-)

 diff --git a/drivers/staging/line6/toneport.c 
 b/drivers/staging/line6/toneport.c
 index af2e7e5..36fe76d 100644
 --- a/drivers/staging/line6/toneport.c
 +++ b/drivers/staging/line6/toneport.c
 @@ -431,11 +431,15 @@ void line6_toneport_disconnect(struct usb_interface 
 *interface)
  {
 struct usb_line6_toneport *toneport;
 u16 idProduct;
 +   struct snd_line6_pcm *line6pcm;

 if (interface == NULL)
 return;

 toneport = usb_get_intfdata(interface);
 +   if (toneport == NULL)
 +   return;
 +
 del_timer_sync(toneport-timer);
 idProduct = le16_to_cpu(toneport-line6.usbdev-descriptor.idProduct);

 @@ -444,13 +448,11 @@ void line6_toneport_disconnect(struct usb_interface 
 *interface)
 device_remove_file(interface-dev, dev_attr_led_green);
 }

 -   if (toneport != NULL) {
 -   struct snd_line6_pcm *line6pcm = toneport-line6.line6pcm;

Didn't look into this but it's the traditional use before NULL
check.  Can't hurt to fix it.

Reviewed-by: Stefan Hajnoczi stefa...@gmail.com
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] staging: line6: toneport.c: Fix for possible null pointer dereference

[Dan Carpenter]

On Mon, May 19, 2014 at 11:39:00PM +0200, Rickard Strandqvist wrote:
 There is otherwise a risk of a possible null pointer dereference.
 
 Was largely found by using a static code analysis program called cppcheck.
 
 Signed-off-by: Rickard Strandqvist rickard_strandqv...@spectrumdigital.se

This one is called from line6_disconnect() and the caller checks that
usb_get_intfdata(interface); returns non-NULL.  So it's messy code but
not a real bug.

For the lustre ones, I don't know if they are real bugs or not.

regards,
dan carpenter

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH] staging: line6: toneport.c: Fix for possible null pointer dereference

[Rickard Strandqvist]

There is otherwise a risk of a possible null pointer dereference.

Was largely found by using a static code analysis program called cppcheck.

Signed-off-by: Rickard Strandqvist 
---
 drivers/staging/line6/toneport.c |   14 --
 1 file changed, 8 insertions(+), 6 deletions(-)

diff --git a/drivers/staging/line6/toneport.c b/drivers/staging/line6/toneport.c
index af2e7e5..36fe76d 100644
--- a/drivers/staging/line6/toneport.c
+++ b/drivers/staging/line6/toneport.c
@@ -431,11 +431,15 @@ void line6_toneport_disconnect(struct usb_interface 
*interface)
 {
struct usb_line6_toneport *toneport;
u16 idProduct;
+   struct snd_line6_pcm *line6pcm;
 
if (interface == NULL)
return;
 
toneport = usb_get_intfdata(interface);
+   if (toneport == NULL)
+   return;
+
del_timer_sync(>timer);
idProduct = le16_to_cpu(toneport->line6.usbdev->descriptor.idProduct);
 
@@ -444,13 +448,11 @@ void line6_toneport_disconnect(struct usb_interface 
*interface)
device_remove_file(>dev, _attr_led_green);
}
 
-   if (toneport != NULL) {
-   struct snd_line6_pcm *line6pcm = toneport->line6.line6pcm;
+   line6pcm = toneport->line6.line6pcm;
 
-   if (line6pcm != NULL) {
-   line6_pcm_release(line6pcm, LINE6_BITS_PCM_MONITOR);
-   line6_pcm_disconnect(line6pcm);
-   }
+   if (line6pcm != NULL) {
+   line6_pcm_release(line6pcm, LINE6_BITS_PCM_MONITOR);
+   line6_pcm_disconnect(line6pcm);
}
 
toneport_destruct(interface);
-- 
1.7.10.4

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH] staging: line6: toneport.c: Fix for possible null pointer dereference

[Rickard Strandqvist]

There is otherwise a risk of a possible null pointer dereference.

Was largely found by using a static code analysis program called cppcheck.

Signed-off-by: Rickard Strandqvist rickard_strandqv...@spectrumdigital.se
---
 drivers/staging/line6/toneport.c |   14 --
 1 file changed, 8 insertions(+), 6 deletions(-)

diff --git a/drivers/staging/line6/toneport.c b/drivers/staging/line6/toneport.c
index af2e7e5..36fe76d 100644
--- a/drivers/staging/line6/toneport.c
+++ b/drivers/staging/line6/toneport.c
@@ -431,11 +431,15 @@ void line6_toneport_disconnect(struct usb_interface 
*interface)
 {
struct usb_line6_toneport *toneport;
u16 idProduct;
+   struct snd_line6_pcm *line6pcm;
 
if (interface == NULL)
return;
 
toneport = usb_get_intfdata(interface);
+   if (toneport == NULL)
+   return;
+
del_timer_sync(toneport-timer);
idProduct = le16_to_cpu(toneport-line6.usbdev-descriptor.idProduct);
 
@@ -444,13 +448,11 @@ void line6_toneport_disconnect(struct usb_interface 
*interface)
device_remove_file(interface-dev, dev_attr_led_green);
}
 
-   if (toneport != NULL) {
-   struct snd_line6_pcm *line6pcm = toneport-line6.line6pcm;
+   line6pcm = toneport-line6.line6pcm;
 
-   if (line6pcm != NULL) {
-   line6_pcm_release(line6pcm, LINE6_BITS_PCM_MONITOR);
-   line6_pcm_disconnect(line6pcm);
-   }
+   if (line6pcm != NULL) {
+   line6_pcm_release(line6pcm, LINE6_BITS_PCM_MONITOR);
+   line6_pcm_disconnect(line6pcm);
}
 
toneport_destruct(interface);
-- 
1.7.10.4

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/