[PATCH 1/2] staging: dgap: fix possible NULL dereference

[PATCH 1/2] staging: dgap: fix possible NULL dereference

[Sudip Mukherjee]

The return pointer from dgap_getword() is used in strcmp() where it is
dereferenced. But dgap_getword() can return NULL.
Lets put a check there and return 0 as error.

Signed-off-by: Sudip Mukherjee <sudip@vectorindia.org>
---
 drivers/staging/dgap/dgap.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/staging/dgap/dgap.c b/drivers/staging/dgap/dgap.c
index 9112dd2..20ba258 100644
--- a/drivers/staging/dgap/dgap.c
+++ b/drivers/staging/dgap/dgap.c
@@ -349,6 +349,8 @@ static int dgap_gettok(char **in)
 
 	if (strstr(dgap_cword, "board")) {
 		w = dgap_getword(in);
+		if (!w)
+			return 0;
 		snprintf(dgap_cword, MAXCWORD, "%s", w);
 		for (t = dgap_brdtype; t->token != 0; t++) {
 			if (!strcmp(w, t->string))
-- 
1.9.1

[PATCH 2/2] staging: dgap: remove unused variable

[Sudip Mukherjee]

These variables were assigned some values but they were never used.
Removed them but kept one call to ioread8() as it might affect the
hardware.

Signed-off-by: Sudip Mukherjee <sudip@vectorindia.org>
---
 drivers/staging/dgap/dgap.c | 6 +-----
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/drivers/staging/dgap/dgap.c b/drivers/staging/dgap/dgap.c
index 20ba258..ee37938 100644
--- a/drivers/staging/dgap/dgap.c
+++ b/drivers/staging/dgap/dgap.c
@@ -1866,7 +1866,6 @@ static int dgap_event(struct board_t *bd)
 	int port;
 	int reason;
 	int modem;
-	int b1;
 
 	if (!bd || bd->magic != DGAP_BOARD_MAGIC)
 		return -EIO;
@@ -1911,7 +1910,7 @@ static int dgap_event(struct board_t *bd)
 		port   = ioread8(event);
 		reason = ioread8(event + 1);
 		modem  = ioread8(event + 2);
-		b1     = ioread8(event + 3);
+		ioread8(event + 3);
 
 		/*
 		 * Make sure the interrupt is valid.
@@ -4570,7 +4569,6 @@ static int dgap_tty_open(struct tty_struct *tty, struct file *file)
  */
 static void dgap_tty_close(struct tty_struct *tty, struct file *file)
 {
-	struct ktermios *ts;
 	struct board_t *bd;
 	struct channel_t *ch;
 	struct un_t *un;
@@ -4591,8 +4589,6 @@ static void dgap_tty_close(struct tty_struct *tty, struct file *file)
 	if (!bd || bd->magic != DGAP_BOARD_MAGIC)
 		return;
 
-	ts = &tty->termios;
-
 	spin_lock_irqsave(&ch->ch_lock, lock_flags);
 
 	/*
-- 
1.9.1

Re: [PATCH 2/2] staging: dgap: remove unused variable

[Dan Carpenter]

On Thu, Sep 10, 2015 at 05:24:59PM +0530, Sudip Mukherjee wrote:
> Removed them but kept one call to ioread8() as it might affect the
> hardware.

Just leave the extra variable until someone has the hardware and can
verify the right fix.  Static checker warnings are good, don't silence
them just to silence them.

regards,
dan carpenter

Re: [PATCH 1/2] staging: dgap: fix possible NULL dereference

[Greg Kroah-Hartman]

On Thu, Sep 10, 2015 at 05:24:58PM +0530, Sudip Mukherjee wrote:
> The return pointer from dgap_getword() is used in strcmp() where it is
> dereferenced. But dgap_getword() can return NULL.
> Lets put a check there and return 0 as error.
> 
> Signed-off-by: Sudip Mukherjee <sudip@vectorindia.org>
> ---
>  drivers/staging/dgap/dgap.c | 2 ++
>  1 file changed, 2 insertions(+)

While this change is all nice and fine, this driver is totally abusing
the firmware kernel interface to read in a "configuration file" and then
parse it within the driver.  That's not ok at all, no tty driver should
ever need this.  Please work to just rip all of that crap out.

thanks,

greg k-h