* [PATCH] jsm_tty: Fix a possible null pointer dereference in two functions
@ 2017-11-29 16:40 SF Markus Elfring
2017-11-29 17:23 ` Joe Perches
0 siblings, 1 reply; 13+ messages in thread
From: SF Markus Elfring @ 2017-11-29 16:40 UTC (permalink / raw)
To: linux-serial, Greg Kroah-Hartman, Guilherme G. Piccoli,
Jiri Slaby, Joe Perches
Cc: LKML, kernel-janitors
From: Markus Elfring <elfring@users.sourceforge.net>
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 <elfring@users.sourceforge.net>
---
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
^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH] jsm_tty: Fix a possible null pointer dereference in two functions
2017-11-29 16:40 [PATCH] jsm_tty: Fix a possible null pointer dereference in two functions SF Markus Elfring
@ 2017-11-29 17:23 ` Joe Perches
2017-11-29 17:35 ` Greg Kroah-Hartman
2017-11-29 19:16 ` [PATCH] jsm_tty: Fix a possible null pointer dereference in two functions Dan Carpenter
0 siblings, 2 replies; 13+ messages in thread
From: Joe Perches @ 2017-11-29 17:23 UTC (permalink / raw)
To: SF Markus Elfring, linux-serial, Greg Kroah-Hartman,
Guilherme G. Piccoli, Jiri Slaby
Cc: LKML, kernel-janitors
On Wed, 2017-11-29 at 17:40 +0100, SF Markus Elfring wrote:
> From: Markus Elfring <elfring@users.sourceforge.net>
> 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 <elfring@users.sourceforge.net>
> ---
> 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)
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] jsm_tty: Fix a possible null pointer dereference in two functions
2017-11-29 17:23 ` Joe Perches
@ 2017-11-29 17:35 ` Greg Kroah-Hartman
2017-11-29 17:51 ` Joe Perches
2017-11-29 19:16 ` [PATCH] jsm_tty: Fix a possible null pointer dereference in two functions Dan Carpenter
1 sibling, 1 reply; 13+ messages in thread
From: Greg Kroah-Hartman @ 2017-11-29 17:35 UTC (permalink / raw)
To: Joe Perches
Cc: SF Markus Elfring, linux-serial, Guilherme G. Piccoli,
Jiri Slaby, LKML, kernel-janitors
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 <elfring@users.sourceforge.net>
> > 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
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] jsm_tty: Fix a possible null pointer dereference in two functions
2017-11-29 17:35 ` Greg Kroah-Hartman
@ 2017-11-29 17:51 ` Joe Perches
2017-11-29 18:05 ` Greg Kroah-Hartman
0 siblings, 1 reply; 13+ messages in thread
From: Joe Perches @ 2017-11-29 17:51 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: SF Markus Elfring, linux-serial, Guilherme G. Piccoli,
Jiri Slaby, LKML, kernel-janitors
On Wed, 2017-11-29 at 17:35 +0000, 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 <elfring@users.sourceforge.net>
> > > 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.
<shrug>
It's pretty unlikely, but it is an actual defect.
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] jsm_tty: Fix a possible null pointer dereference in two functions
2017-11-29 17:51 ` Joe Perches
@ 2017-11-29 18:05 ` Greg Kroah-Hartman
2017-11-29 18:16 ` Joe Perches
2017-11-29 18:19 ` SF Markus Elfring
0 siblings, 2 replies; 13+ messages in thread
From: Greg Kroah-Hartman @ 2017-11-29 18:05 UTC (permalink / raw)
To: Joe Perches
Cc: SF Markus Elfring, linux-serial, Guilherme G. Piccoli,
Jiri Slaby, LKML, kernel-janitors
On Wed, Nov 29, 2017 at 09:51:36AM -0800, Joe Perches wrote:
> On Wed, 2017-11-29 at 17:35 +0000, 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 <elfring@users.sourceforge.net>
> > > > 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.
>
> <shrug>
>
> 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
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] jsm_tty: Fix a possible null pointer dereference in two functions
2017-11-29 18:05 ` Greg Kroah-Hartman
@ 2017-11-29 18:16 ` Joe Perches
2017-11-29 18:19 ` SF Markus Elfring
1 sibling, 0 replies; 13+ messages in thread
From: Joe Perches @ 2017-11-29 18:16 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: SF Markus Elfring, linux-serial, Guilherme G. Piccoli,
Jiri Slaby, LKML, kernel-janitors
On Wed, 2017-11-29 at 18:05 +0000, 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 +0000, 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 <elfring@users.sourceforge.net>
> > > > > 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.
> >
> > <shrug>
> >
> > 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
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: jsm_tty: Fix a possible null pointer dereference in two functions
2017-11-29 18:05 ` Greg Kroah-Hartman
2017-11-29 18:16 ` Joe Perches
@ 2017-11-29 18:19 ` SF Markus Elfring
2017-11-30 6:41 ` Jiri Slaby
2017-12-06 16:16 ` Guilherme G. Piccoli
1 sibling, 2 replies; 13+ messages in thread
From: SF Markus Elfring @ 2017-11-29 18:19 UTC (permalink / raw)
To: Greg Kroah-Hartman, linux-serial
Cc: Joe Perches, Guilherme G. Piccoli, Jiri Slaby, LKML, kernel-janitors
>> 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
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] jsm_tty: Fix a possible null pointer dereference in two functions
2017-11-29 17:23 ` Joe Perches
2017-11-29 17:35 ` Greg Kroah-Hartman
@ 2017-11-29 19:16 ` Dan Carpenter
1 sibling, 0 replies; 13+ messages in thread
From: Dan Carpenter @ 2017-11-29 19:16 UTC (permalink / raw)
To: Joe Perches
Cc: SF Markus Elfring, linux-serial, Greg Kroah-Hartman,
Guilherme G. Piccoli, Jiri Slaby, LKML, kernel-janitors
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 <elfring@users.sourceforge.net>
> > 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
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: jsm_tty: Fix a possible null pointer dereference in two functions
2017-11-29 18:19 ` SF Markus Elfring
@ 2017-11-30 6:41 ` Jiri Slaby
2017-12-06 16:16 ` Guilherme G. Piccoli
1 sibling, 0 replies; 13+ messages in thread
From: Jiri Slaby @ 2017-11-30 6:41 UTC (permalink / raw)
To: SF Markus Elfring, Greg Kroah-Hartman, linux-serial
Cc: Guilherme G. Piccoli, Joe Perches, kernel-janitors, LKML
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
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: jsm_tty: Fix a possible null pointer dereference in two functions
2017-11-29 18:19 ` SF Markus Elfring
2017-11-30 6:41 ` Jiri Slaby
@ 2017-12-06 16:16 ` Guilherme G. Piccoli
2017-12-16 6:27 ` jsm_tty: Deletion of a null pointer check in two functions? SF Markus Elfring
1 sibling, 1 reply; 13+ messages in thread
From: Guilherme G. Piccoli @ 2017-12-06 16:16 UTC (permalink / raw)
To: SF Markus Elfring, linux-serial
Cc: Greg Kroah-Hartman, Joe Perches, Jiri Slaby, LKML, kernel-janitors
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
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: jsm_tty: Deletion of a null pointer check in two functions?
2017-12-06 16:16 ` Guilherme G. Piccoli
@ 2017-12-16 6:27 ` SF Markus Elfring
2017-12-18 14:36 ` Guilherme G. Piccoli
0 siblings, 1 reply; 13+ messages in thread
From: SF Markus Elfring @ 2017-12-16 6:27 UTC (permalink / raw)
To: Guilherme G. Piccoli, linux-serial
Cc: Greg Kroah-Hartman, Joe Perches, Jiri Slaby, LKML, kernel-janitors
> Thanks for the fix.
Thanks for your positive feedback.
> I was on vacation - but now seeing all the analysis made here,
I assume that special communication settings could trigger
corresponding consequences for the discussed source code adjustment.
> if "ch" can't be NULL then please go ahead and remove the check =)
Would you dare to convert this request into a concrete patch?
Regards,
Markus
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: jsm_tty: Deletion of a null pointer check in two functions?
2017-12-16 6:27 ` jsm_tty: Deletion of a null pointer check in two functions? SF Markus Elfring
@ 2017-12-18 14:36 ` Guilherme G. Piccoli
2017-12-18 17:14 ` SF Markus Elfring
0 siblings, 1 reply; 13+ messages in thread
From: Guilherme G. Piccoli @ 2017-12-18 14:36 UTC (permalink / raw)
To: SF Markus Elfring
Cc: linux-serial, Greg Kroah-Hartman, Joe Perches, Jiri Slaby, LKML,
kernel-janitors
On 12/16/2017 04:27 AM, SF Markus Elfring wrote:
>> Thanks for the fix.
>
> Thanks for your positive feedback.
>
>
>> I was on vacation - but now seeing all the analysis made here,
>
> I assume that special communication settings could trigger
> corresponding consequences for the discussed source code adjustment.
>
>
>> if "ch" can't be NULL then please go ahead and remove the check =)
>
> Would you dare to convert this request into a concrete patch?
For me, it's OK if you send the patch or I can do it myself.
It's a minor patch...but it's correct the way I see it.
So, what do you prefer? Send it yourself, or want me to send
it with your sign-off too? (since was your idea).
Thanks,
Guilherme
>
> Regards,
> Markus
>
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: jsm_tty: Deletion of a null pointer check in two functions?
2017-12-18 14:36 ` Guilherme G. Piccoli
@ 2017-12-18 17:14 ` SF Markus Elfring
0 siblings, 0 replies; 13+ messages in thread
From: SF Markus Elfring @ 2017-12-18 17:14 UTC (permalink / raw)
To: Guilherme G. Piccoli, linux-serial
Cc: Greg Kroah-Hartman, Joe Perches, Jiri Slaby, LKML, kernel-janitors
> So, what do you prefer? Send it yourself,
I became concerned that “my patch” would not get picked up because of
Greg's communication settings (for my initial messages when they do not get
resent by other contributors).
> or want me to send it with your sign-off too? (since was your idea).
I would prefer this variant because I assume that the chances for integration
are higher for the discussed small source code adjustment.
I guess that the tag “Suggested-by” would be sufficient in this case.
Regards,
Markus
^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2017-12-18 17:14 UTC | newest]
Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-11-29 16:40 [PATCH] jsm_tty: Fix a possible null pointer dereference in two functions SF Markus Elfring
2017-11-29 17:23 ` Joe Perches
2017-11-29 17:35 ` Greg Kroah-Hartman
2017-11-29 17:51 ` Joe Perches
2017-11-29 18:05 ` Greg Kroah-Hartman
2017-11-29 18:16 ` Joe Perches
2017-11-29 18:19 ` SF Markus Elfring
2017-11-30 6:41 ` Jiri Slaby
2017-12-06 16:16 ` Guilherme G. Piccoli
2017-12-16 6:27 ` jsm_tty: Deletion of a null pointer check in two functions? SF Markus Elfring
2017-12-18 14:36 ` Guilherme G. Piccoli
2017-12-18 17:14 ` SF Markus Elfring
2017-11-29 19:16 ` [PATCH] jsm_tty: Fix a possible null pointer dereference in two functions Dan Carpenter
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).