Patchwork AW: [PATCH] ISDN cmx: Avoid potential NULL deref in dsp_cmx_send_member() and shrink code size.

login
register
mail settings
Submitter Andreas.Eversberg
Date Dec. 20, 2010, 11:26 a.m.
Message ID <B16317F2B1592B4492FC414114171CE604B08AEA@FLBVEXCH01.versatel.local>
Download mbox | patch
Permalink /patch/76199/
State RFC
Delegated to: David Miller
Headers show

Comments

Andreas.Eversberg - Dec. 20, 2010, 11:26 a.m.
hi jesper,

thanx for finding the bug. i think the right solution to solve the problem would be:

                if (dsp->conf && dsp->conf->software && dsp->conf->hardware)
                        tx_data_only = 1;
->              if (dsp->echo.software && dsp->echo.hardware)
                        tx_data_only = 1;

this is how it looks in the 'socket' branch of mISDN git respository. it has been fixed already. but i cannot tell in which commit. my current head is this commit:
commit 45a51eed1c554a4891b48b88c270f4f95bd21df0

what branch do you use? 

regards,

andreas


-----Urspr√ľngliche Nachricht-----
Von: Jesper Juhl [mailto:jj@chaosbits.net] 
Gesendet: Samstag, 18. Dezember 2010 23:34
An: Karsten Keil
Cc: David S. Miller; Julia Lawall; Tejun Heo; netdev@vger.kernel.org; linux-kernel@vger.kernel.org; Andreas Eversberg
Betreff: [PATCH] ISDN cmx: Avoid potential NULL deref in dsp_cmx_send_member() and shrink code size.

Hi there,

In drivers/isdn/mISDN/dsp_cmx.c::dsp_cmx_send_member() we currently have 
this code:

           if (dsp->conf && dsp->conf->software && dsp->conf->hardware)
                   tx_data_only = 1;
           if (dsp->conf->software && dsp->echo.hardware)
                   tx_data_only = 1;

The first line implies that 'dsp->conf' may be NULL. If it is, then the 
third line will dereference a NULL pointer.

This patch reworks the code so that we avoid the potential NULL deref.
It also has the added benefit that the object file size shrinks a bit.

before:
   text    data     bss     dec     hex filename
  18840     112    5784   24736    60a0 drivers/isdn/mISDN/dsp_cmx.o
after:
   text    data     bss     dec     hex filename
  18816     112    5776   24704    6080 drivers/isdn/mISDN/dsp_cmx.o


Signed-off-by: Jesper Juhl <jj@chaosbits.net>
---
 dsp_cmx.c |    7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

 compile tested only
Jesper Juhl - Dec. 20, 2010, 10:10 p.m.
On Mon, 20 Dec 2010, Andreas.Eversberg wrote:

> hi jesper,
> 
> thanx for finding the bug. i think the right solution to solve the problem would be:
> 
>                 if (dsp->conf && dsp->conf->software && dsp->conf->hardware)
>                         tx_data_only = 1;
> ->              if (dsp->echo.software && dsp->echo.hardware)
>                         tx_data_only = 1;
> 
> this is how it looks in the 'socket' branch of mISDN git respository. it has been fixed already. but i cannot tell in which commit. my current head is this commit:
> commit 45a51eed1c554a4891b48b88c270f4f95bd21df0
> 
I'm not familiar enough with the code to determine if my fix or the one 
you propose is the right one. My fix is functionally equivalent to what 
was there before (minus the NULL deref), yours is not. I'll let someone 
more knowledgeable about the ISDN code determine what is the right fix.

> what branch do you use? 
> 
I'm working against Linus' kernel as of today.

Patch

diff --git a/drivers/isdn/mISDN/dsp_cmx.c b/drivers/isdn/mISDN/dsp_cmx.c
index 76d9e67..f76f595 100644
--- a/drivers/isdn/mISDN/dsp_cmx.c
+++ b/drivers/isdn/mISDN/dsp_cmx.c
@@ -1326,10 +1326,9 @@  dsp_cmx_send_member(struct dsp *dsp, int len, s32 *c, int members)
 			dsp->last_tx = 0;
 			return;
 		}
-		if (dsp->conf && dsp->conf->software && dsp->conf->hardware)
-			tx_data_only = 1;
-		if (dsp->conf->software && dsp->echo.hardware)
-			tx_data_only = 1;
+		if (dsp->conf && dsp->conf->software)
+			if (dsp->conf->hardware || dsp->echo.hardware)
+				tx_data_only = 1;
 	}
 
 #ifdef CMX_DEBUG