Re: jsm_tty: Fix a possible null pointer dereference in two functions
On 11/29/2017 04:19 PM, SF Markus Elfring wrote: >>> It's pretty unlikely, but it is an actual defect. >> >> No it is not, those variables will never be set to NULL, >> so this can never be triggered. Walk up the call chain. > > If the involved software developers are convinced about the validity > of this pointer: > > How do you think about to delete the following condition check > instead in the discussed function implementations? > > if (!ch) > return; > > > Regards, > Markus > Thanks for the fix. I was on vacation - but now seeing all the analysis made here, if "ch" can't be NULL then please go ahead and remove the check =) I observed that this check comes from before Git, so really ancient code... Cheers, Guilherme
Re: jsm_tty: Fix a possible null pointer dereference in two functions
On 11/29/2017, 07:19 PM, SF Markus Elfring wrote: >>> It's pretty unlikely, but it is an actual defect. >> >> No it is not, those variables will never be set to NULL, >> so this can never be triggered. Walk up the call chain. > > If the involved software developers are convinced about the validity > of this pointer: > > How do you think about to delete the following condition check > instead in the discussed function implementations? > > if (!ch) > return; ACK thanks, -- js suse labs
Re: [PATCH] jsm_tty: Fix a possible null pointer dereference in two functions
On Wed, Nov 29, 2017 at 09:23:07AM -0800, Joe Perches wrote: > On Wed, 2017-11-29 at 17:40 +0100, SF Markus Elfring wrote: > > From: Markus Elfring > > Date: Wed, 29 Nov 2017 17:30:36 +0100 > > > > Move two debug messages so that a null pointer access can not happen > > for the variable "ch" in these functions. > > An actual defect fix! > > Here you could probably cc stable too. > These aren't real bugs because "ch" is never NULL. regards, dan carpenter
Re: jsm_tty: Fix a possible null pointer dereference in two functions
>> It's pretty unlikely, but it is an actual defect. > > No it is not, those variables will never be set to NULL, > so this can never be triggered. Walk up the call chain. If the involved software developers are convinced about the validity of this pointer: How do you think about to delete the following condition check instead in the discussed function implementations? if (!ch) return; Regards, Markus
Re: [PATCH] jsm_tty: Fix a possible null pointer dereference in two functions
On Wed, 2017-11-29 at 18:05 +, Greg Kroah-Hartman wrote: > On Wed, Nov 29, 2017 at 09:51:36AM -0800, Joe Perches wrote: > > On Wed, 2017-11-29 at 17:35 +, Greg Kroah-Hartman wrote: > > > On Wed, Nov 29, 2017 at 09:23:07AM -0800, Joe Perches wrote: > > > > On Wed, 2017-11-29 at 17:40 +0100, SF Markus Elfring wrote: > > > > > From: Markus Elfring > > > > > Date: Wed, 29 Nov 2017 17:30:36 +0100 > > > > > > > > > > Move two debug messages so that a null pointer access can not happen > > > > > for the variable "ch" in these functions. > > > > > > > > An actual defect fix! > > > > > > Nope, not at all, this does not "fix" anything. > > > > Well, I believe it does in unusual cases like a > > CONFIG_DYNAMIC_DEBUG when this is enabled by an > > odd +p in the dynamic debug control file. > > > > > > Here you could probably cc stable too. > > > > > > Nope, not worth it. > > > > > > > > It's pretty unlikely, but it is an actual defect. > > No it is not, those variables will never be set to NULL, so this can > never be triggered. Walk up the call chain. Right you are. Local analysis isn't enough. The code could/should be removed, but it's not a defect. cheers, Joe
Re: [PATCH] jsm_tty: Fix a possible null pointer dereference in two functions
On Wed, Nov 29, 2017 at 09:51:36AM -0800, Joe Perches wrote: > On Wed, 2017-11-29 at 17:35 +, Greg Kroah-Hartman wrote: > > On Wed, Nov 29, 2017 at 09:23:07AM -0800, Joe Perches wrote: > > > On Wed, 2017-11-29 at 17:40 +0100, SF Markus Elfring wrote: > > > > From: Markus Elfring > > > > Date: Wed, 29 Nov 2017 17:30:36 +0100 > > > > > > > > Move two debug messages so that a null pointer access can not happen > > > > for the variable "ch" in these functions. > > > > > > An actual defect fix! > > > > Nope, not at all, this does not "fix" anything. > > Well, I believe it does in unusual cases like a > CONFIG_DYNAMIC_DEBUG when this is enabled by an > odd +p in the dynamic debug control file. > > > > Here you could probably cc stable too. > > > > Nope, not worth it. > > > > It's pretty unlikely, but it is an actual defect. No it is not, those variables will never be set to NULL, so this can never be triggered. Walk up the call chain. greg k-h
Re: [PATCH] jsm_tty: Fix a possible null pointer dereference in two functions
On Wed, 2017-11-29 at 17:35 +, Greg Kroah-Hartman wrote: > On Wed, Nov 29, 2017 at 09:23:07AM -0800, Joe Perches wrote: > > On Wed, 2017-11-29 at 17:40 +0100, SF Markus Elfring wrote: > > > From: Markus Elfring > > > Date: Wed, 29 Nov 2017 17:30:36 +0100 > > > > > > Move two debug messages so that a null pointer access can not happen > > > for the variable "ch" in these functions. > > > > An actual defect fix! > > Nope, not at all, this does not "fix" anything. Well, I believe it does in unusual cases like a CONFIG_DYNAMIC_DEBUG when this is enabled by an odd +p in the dynamic debug control file. > > Here you could probably cc stable too. > > Nope, not worth it. It's pretty unlikely, but it is an actual defect.
Re: [PATCH] jsm_tty: Fix a possible null pointer dereference in two functions
On Wed, 2017-11-29 at 17:40 +0100, SF Markus Elfring wrote:
> From: Markus Elfring
> Date: Wed, 29 Nov 2017 17:30:36 +0100
>
> Move two debug messages so that a null pointer access can not happen
> for the variable "ch" in these functions.
An actual defect fix!
Here you could probably cc stable too.
>
> This issue was detected by using the Coccinelle software.
>
> Fixes: 669fef464468d3f02d60a5cf725fc097e03c5cb8 ("serial: jsm: Convert
> jsm_printk to jsm_dbg")
>
> Signed-off-by: Markus Elfring
> ---
> drivers/tty/serial/jsm/jsm_tty.c | 5 ++---
> 1 file changed, 2 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/tty/serial/jsm/jsm_tty.c
> b/drivers/tty/serial/jsm/jsm_tty.c
> index 469927d37b41..a34eed7344e5 100644
> --- a/drivers/tty/serial/jsm/jsm_tty.c
> +++ b/drivers/tty/serial/jsm/jsm_tty.c
> @@ -521,11 +521,10 @@ void jsm_input(struct jsm_channel *ch)
> int s = 0;
> int i = 0;
>
> - jsm_dbg(READ, &ch->ch_bd->pci_dev, "start\n");
> -
> if (!ch)
> return;
>
> + jsm_dbg(READ, &ch->ch_bd->pci_dev, "start\n");
> port = &ch->uart_port.state->port;
> tp = port->tty;
>
> @@ -647,10 +646,10 @@ static void jsm_carrier(struct jsm_channel *ch)
> int virt_carrier = 0;
> int phys_carrier = 0;
>
> - jsm_dbg(CARR, &ch->ch_bd->pci_dev, "start\n");
> if (!ch)
> return;
>
> + jsm_dbg(CARR, &ch->ch_bd->pci_dev, "start\n");
> bd = ch->ch_bd;
>
> if (!bd)
Re: [PATCH] jsm_tty: Fix a possible null pointer dereference in two functions
On Wed, Nov 29, 2017 at 09:23:07AM -0800, Joe Perches wrote: > On Wed, 2017-11-29 at 17:40 +0100, SF Markus Elfring wrote: > > From: Markus Elfring > > Date: Wed, 29 Nov 2017 17:30:36 +0100 > > > > Move two debug messages so that a null pointer access can not happen > > for the variable "ch" in these functions. > > An actual defect fix! Nope, not at all, this does not "fix" anything. > Here you could probably cc stable too. Nope, not worth it. greg k-h
[PATCH] jsm_tty: Fix a possible null pointer dereference in two functions
From: Markus Elfring
Date: Wed, 29 Nov 2017 17:30:36 +0100
Move two debug messages so that a null pointer access can not happen
for the variable "ch" in these functions.
This issue was detected by using the Coccinelle software.
Fixes: 669fef464468d3f02d60a5cf725fc097e03c5cb8 ("serial: jsm: Convert
jsm_printk to jsm_dbg")
Signed-off-by: Markus Elfring
---
drivers/tty/serial/jsm/jsm_tty.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/drivers/tty/serial/jsm/jsm_tty.c b/drivers/tty/serial/jsm/jsm_tty.c
index 469927d37b41..a34eed7344e5 100644
--- a/drivers/tty/serial/jsm/jsm_tty.c
+++ b/drivers/tty/serial/jsm/jsm_tty.c
@@ -521,11 +521,10 @@ void jsm_input(struct jsm_channel *ch)
int s = 0;
int i = 0;
- jsm_dbg(READ, &ch->ch_bd->pci_dev, "start\n");
-
if (!ch)
return;
+ jsm_dbg(READ, &ch->ch_bd->pci_dev, "start\n");
port = &ch->uart_port.state->port;
tp = port->tty;
@@ -647,10 +646,10 @@ static void jsm_carrier(struct jsm_channel *ch)
int virt_carrier = 0;
int phys_carrier = 0;
- jsm_dbg(CARR, &ch->ch_bd->pci_dev, "start\n");
if (!ch)
return;
+ jsm_dbg(CARR, &ch->ch_bd->pci_dev, "start\n");
bd = ch->ch_bd;
if (!bd)
--
2.15.0
