[PATCH 1/5] [media] cx22700: Fix potential buffer overflow

All of lore.kernel.org
 help / color / mirror / Atom feed

* [PATCH 1/5] [media] cx22700: Fix potential buffer overflow
@ 2014-11-05 12:03 Mauro Carvalho Chehab
  2014-11-05 12:03 ` [PATCH 2/5] [media] cx24110: Fix a spatch warning Mauro Carvalho Chehab
                   ` (3 more replies)
  0 siblings, 4 replies; 7+ messages in thread
From: Mauro Carvalho Chehab @ 2014-11-05 12:03 UTC (permalink / raw)
  To: Linux Media Mailing List; +Cc: Mauro Carvalho Chehab, Mauro Carvalho Chehab

As new FEC types were added, we need a check to avoid overflows:
	drivers/media/dvb-frontends/cx22700.c:172 cx22700_set_tps() error: buffer overflow 'fec_tab' 6 <= 6
	drivers/media/dvb-frontends/cx22700.c:173 cx22700_set_tps() error: buffer overflow 'fec_tab' 6 <= 6

Signed-off-by: Mauro Carvalho Chehab <mchehab@osg.samsung.com>

diff --git a/drivers/media/dvb-frontends/cx22700.c b/drivers/media/dvb-frontends/cx22700.c
index 3d399d9a6343..86563260d0f2 100644
--- a/drivers/media/dvb-frontends/cx22700.c
+++ b/drivers/media/dvb-frontends/cx22700.c
@@ -169,6 +169,9 @@ static int cx22700_set_tps(struct cx22700_state *state,
 
 	cx22700_writereg (state, 0x04, val);
 
+	if (p->code_rate_HP - FEC_1_2 >= sizeof(fec_tab) ||
+	    p->code_rate_LP - FEC_1_2 >= sizeof(fec_tab))
+		return -EINVAL;
 	val = fec_tab[p->code_rate_HP - FEC_1_2] << 3;
 	val |= fec_tab[p->code_rate_LP - FEC_1_2];
 
-- 
1.9.3


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH 2/5] [media] cx24110: Fix a spatch warning
  2014-11-05 12:03 [PATCH 1/5] [media] cx22700: Fix potential buffer overflow Mauro Carvalho Chehab
@ 2014-11-05 12:03 ` Mauro Carvalho Chehab
  2014-11-05 12:21   ` Hans Verkuil
  2014-11-05 12:03 ` [PATCH 3/5] [media] cx24110: Fix whitespaces at cx24110_set_fec() Mauro Carvalho Chehab
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 7+ messages in thread
From: Mauro Carvalho Chehab @ 2014-11-05 12:03 UTC (permalink / raw)
  To: Linux Media Mailing List; +Cc: Mauro Carvalho Chehab, Mauro Carvalho Chehab

This is actually a false positive:
	drivers/media/dvb-frontends/cx24110.c:210 cx24110_set_fec() error: buffer overflow 'rate' 7 <= 8

But fixing it is easy: just ensure that the table size will be
limited to FEC_AUTO.

While here, fix spacing on the affected lines.

Signed-off-by: Mauro Carvalho Chehab <mchehab@osg.samsung.com>

diff --git a/drivers/media/dvb-frontends/cx24110.c b/drivers/media/dvb-frontends/cx24110.c
index 95b981cd7115..e78e7893e8aa 100644
--- a/drivers/media/dvb-frontends/cx24110.c
+++ b/drivers/media/dvb-frontends/cx24110.c
@@ -181,16 +181,16 @@ static int cx24110_set_fec (struct cx24110_state* state, fe_code_rate_t fec)
 {
 /* fixme (low): error handling */
 
-	static const int rate[]={-1,1,2,3,5,7,-1};
-	static const int g1[]={-1,0x01,0x02,0x05,0x15,0x45,-1};
-	static const int g2[]={-1,0x01,0x03,0x06,0x1a,0x7a,-1};
+	static const int rate[FEC_AUTO] = {-1,    1,    2,    3,    5,    7, -1};
+	static const int g1[FEC_AUTO]   = {-1, 0x01, 0x02, 0x05, 0x15, 0x45, -1};
+	static const int g2[FEC_AUTO]   = {-1, 0x01, 0x03, 0x06, 0x1a, 0x7a, -1};
 
 	/* Well, the AutoAcq engine of the cx24106 and 24110 automatically
 	   searches all enabled viterbi rates, and can handle non-standard
 	   rates as well. */
 
-	if (fec>FEC_AUTO)
-		fec=FEC_AUTO;
+	if (fec > FEC_AUTO)
+		fec = FEC_AUTO;
 
 	if (fec==FEC_AUTO) { /* (re-)establish AutoAcq behaviour */
 		cx24110_writereg(state,0x37,cx24110_readreg(state,0x37)&0xdf);
-- 
1.9.3


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH 2/5] [media] cx24110: Fix a spatch warning
  2014-11-05 12:03 ` [PATCH 2/5] [media] cx24110: Fix a spatch warning Mauro Carvalho Chehab
@ 2014-11-05 12:21   ` Hans Verkuil
  2014-11-05 12:41     ` Mauro Carvalho Chehab
  0 siblings, 1 reply; 7+ messages in thread
From: Hans Verkuil @ 2014-11-05 12:21 UTC (permalink / raw)
  To: Mauro Carvalho Chehab, Linux Media Mailing List; +Cc: Mauro Carvalho Chehab

spatch or smatch? I assume smatch :-)

BTW, I've just added smatch support to the daily build.

Regards,

	Hans

On 11/05/14 13:03, Mauro Carvalho Chehab wrote:
> This is actually a false positive:
> 	drivers/media/dvb-frontends/cx24110.c:210 cx24110_set_fec() error: buffer overflow 'rate' 7 <= 8
> 
> But fixing it is easy: just ensure that the table size will be
> limited to FEC_AUTO.
> 
> While here, fix spacing on the affected lines.
> 
> Signed-off-by: Mauro Carvalho Chehab <mchehab@osg.samsung.com>
> 
> diff --git a/drivers/media/dvb-frontends/cx24110.c b/drivers/media/dvb-frontends/cx24110.c
> index 95b981cd7115..e78e7893e8aa 100644
> --- a/drivers/media/dvb-frontends/cx24110.c
> +++ b/drivers/media/dvb-frontends/cx24110.c
> @@ -181,16 +181,16 @@ static int cx24110_set_fec (struct cx24110_state* state, fe_code_rate_t fec)
>  {
>  /* fixme (low): error handling */
>  
> -	static const int rate[]={-1,1,2,3,5,7,-1};
> -	static const int g1[]={-1,0x01,0x02,0x05,0x15,0x45,-1};
> -	static const int g2[]={-1,0x01,0x03,0x06,0x1a,0x7a,-1};
> +	static const int rate[FEC_AUTO] = {-1,    1,    2,    3,    5,    7, -1};
> +	static const int g1[FEC_AUTO]   = {-1, 0x01, 0x02, 0x05, 0x15, 0x45, -1};
> +	static const int g2[FEC_AUTO]   = {-1, 0x01, 0x03, 0x06, 0x1a, 0x7a, -1};
>  
>  	/* Well, the AutoAcq engine of the cx24106 and 24110 automatically
>  	   searches all enabled viterbi rates, and can handle non-standard
>  	   rates as well. */
>  
> -	if (fec>FEC_AUTO)
> -		fec=FEC_AUTO;
> +	if (fec > FEC_AUTO)
> +		fec = FEC_AUTO;
>  
>  	if (fec==FEC_AUTO) { /* (re-)establish AutoAcq behaviour */
>  		cx24110_writereg(state,0x37,cx24110_readreg(state,0x37)&0xdf);
> 


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH 2/5] [media] cx24110: Fix a spatch warning
  2014-11-05 12:21   ` Hans Verkuil
@ 2014-11-05 12:41     ` Mauro Carvalho Chehab
  0 siblings, 0 replies; 7+ messages in thread
From: Mauro Carvalho Chehab @ 2014-11-05 12:41 UTC (permalink / raw)
  To: Hans Verkuil; +Cc: Linux Media Mailing List, Mauro Carvalho Chehab

Em Wed, 05 Nov 2014 13:21:31 +0100
Hans Verkuil <hverkuil@xs4all.nl> escreveu:

> spatch or smatch? I assume smatch :-)

Yeah, typo... both are tools used for static code analizers... very easy to
type it wrong ;)

> 
> BTW, I've just added smatch support to the daily build.

Good!

I'll fix some more smatch errors today. There are some false positives
there, but some seems to be real issues.
> 
> Regards,
> 
> 	Hans
> 
> On 11/05/14 13:03, Mauro Carvalho Chehab wrote:
> > This is actually a false positive:
> > 	drivers/media/dvb-frontends/cx24110.c:210 cx24110_set_fec() error: buffer overflow 'rate' 7 <= 8
> > 
> > But fixing it is easy: just ensure that the table size will be
> > limited to FEC_AUTO.
> > 
> > While here, fix spacing on the affected lines.
> > 
> > Signed-off-by: Mauro Carvalho Chehab <mchehab@osg.samsung.com>
> > 
> > diff --git a/drivers/media/dvb-frontends/cx24110.c b/drivers/media/dvb-frontends/cx24110.c
> > index 95b981cd7115..e78e7893e8aa 100644
> > --- a/drivers/media/dvb-frontends/cx24110.c
> > +++ b/drivers/media/dvb-frontends/cx24110.c
> > @@ -181,16 +181,16 @@ static int cx24110_set_fec (struct cx24110_state* state, fe_code_rate_t fec)
> >  {
> >  /* fixme (low): error handling */
> >  
> > -	static const int rate[]={-1,1,2,3,5,7,-1};
> > -	static const int g1[]={-1,0x01,0x02,0x05,0x15,0x45,-1};
> > -	static const int g2[]={-1,0x01,0x03,0x06,0x1a,0x7a,-1};
> > +	static const int rate[FEC_AUTO] = {-1,    1,    2,    3,    5,    7, -1};
> > +	static const int g1[FEC_AUTO]   = {-1, 0x01, 0x02, 0x05, 0x15, 0x45, -1};
> > +	static const int g2[FEC_AUTO]   = {-1, 0x01, 0x03, 0x06, 0x1a, 0x7a, -1};
> >  
> >  	/* Well, the AutoAcq engine of the cx24106 and 24110 automatically
> >  	   searches all enabled viterbi rates, and can handle non-standard
> >  	   rates as well. */
> >  
> > -	if (fec>FEC_AUTO)
> > -		fec=FEC_AUTO;
> > +	if (fec > FEC_AUTO)
> > +		fec = FEC_AUTO;
> >  
> >  	if (fec==FEC_AUTO) { /* (re-)establish AutoAcq behaviour */
> >  		cx24110_writereg(state,0x37,cx24110_readreg(state,0x37)&0xdf);
> > 
> 

^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH 3/5] [media] cx24110: Fix whitespaces at cx24110_set_fec()
  2014-11-05 12:03 [PATCH 1/5] [media] cx22700: Fix potential buffer overflow Mauro Carvalho Chehab
  2014-11-05 12:03 ` [PATCH 2/5] [media] cx24110: Fix a spatch warning Mauro Carvalho Chehab
@ 2014-11-05 12:03 ` Mauro Carvalho Chehab
  2014-11-05 12:03 ` [PATCH 4/5] [media] cx23110: Fix return code for cx24110_set_fec() Mauro Carvalho Chehab
  2014-11-05 12:03 ` [PATCH 5/5] [media] cx24110: Simplify error handling at cx24110_set_fec() Mauro Carvalho Chehab
  3 siblings, 0 replies; 7+ messages in thread
From: Mauro Carvalho Chehab @ 2014-11-05 12:03 UTC (permalink / raw)
  To: Linux Media Mailing List; +Cc: Mauro Carvalho Chehab, Mauro Carvalho Chehab

It is hard to read what's there, because it doesn't follow the
CodingStyle.

Add missing whitespaces to split function arguments.

No functional changes.

Signed-off-by: Mauro Carvalho Chehab <mchehab@osg.samsung.com>

diff --git a/drivers/media/dvb-frontends/cx24110.c b/drivers/media/dvb-frontends/cx24110.c
index e78e7893e8aa..4f5c992afe67 100644
--- a/drivers/media/dvb-frontends/cx24110.c
+++ b/drivers/media/dvb-frontends/cx24110.c
@@ -192,28 +192,29 @@ static int cx24110_set_fec (struct cx24110_state* state, fe_code_rate_t fec)
 	if (fec > FEC_AUTO)
 		fec = FEC_AUTO;
 
-	if (fec==FEC_AUTO) { /* (re-)establish AutoAcq behaviour */
-		cx24110_writereg(state,0x37,cx24110_readreg(state,0x37)&0xdf);
+	if (fec == FEC_AUTO) { /* (re-)establish AutoAcq behaviour */
+		cx24110_writereg(state, 0x37, cx24110_readreg(state, 0x37) & 0xdf);
 		/* clear AcqVitDis bit */
-		cx24110_writereg(state,0x18,0xae);
+		cx24110_writereg(state, 0x18, 0xae);
 		/* allow all DVB standard code rates */
-		cx24110_writereg(state,0x05,(cx24110_readreg(state,0x05)&0xf0)|0x3);
+		cx24110_writereg(state, 0x05, (cx24110_readreg(state, 0x05) & 0xf0) | 0x3);
 		/* set nominal Viterbi rate 3/4 */
-		cx24110_writereg(state,0x22,(cx24110_readreg(state,0x22)&0xf0)|0x3);
+		cx24110_writereg(state, 0x22, (cx24110_readreg(state, 0x22) & 0xf0) | 0x3);
 		/* set current Viterbi rate 3/4 */
-		cx24110_writereg(state,0x1a,0x05); cx24110_writereg(state,0x1b,0x06);
+		cx24110_writereg(state, 0x1a, 0x05);
+		cx24110_writereg(state, 0x1b, 0x06);
 		/* set the puncture registers for code rate 3/4 */
 		return 0;
 	} else {
-		cx24110_writereg(state,0x37,cx24110_readreg(state,0x37)|0x20);
+		cx24110_writereg(state, 0x37, cx24110_readreg(state, 0x37) | 0x20);
 		/* set AcqVitDis bit */
-		if(rate[fec]>0) {
-			cx24110_writereg(state,0x05,(cx24110_readreg(state,0x05)&0xf0)|rate[fec]);
+		if (rate[fec] > 0) {
+			cx24110_writereg(state, 0x05, (cx24110_readreg(state, 0x05) & 0xf0) | rate[fec]);
 			/* set nominal Viterbi rate */
-			cx24110_writereg(state,0x22,(cx24110_readreg(state,0x22)&0xf0)|rate[fec]);
+			cx24110_writereg(state, 0x22, (cx24110_readreg(state, 0x22) & 0xf0) | rate[fec]);
 			/* set current Viterbi rate */
-			cx24110_writereg(state,0x1a,g1[fec]);
-			cx24110_writereg(state,0x1b,g2[fec]);
+			cx24110_writereg(state, 0x1a, g1[fec]);
+			cx24110_writereg(state, 0x1b, g2[fec]);
 			/* not sure if this is the right way: I always used AutoAcq mode */
 	   } else
 		   return -EOPNOTSUPP;
-- 
1.9.3


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH 4/5] [media] cx23110: Fix return code for cx24110_set_fec()
  2014-11-05 12:03 [PATCH 1/5] [media] cx22700: Fix potential buffer overflow Mauro Carvalho Chehab
  2014-11-05 12:03 ` [PATCH 2/5] [media] cx24110: Fix a spatch warning Mauro Carvalho Chehab
  2014-11-05 12:03 ` [PATCH 3/5] [media] cx24110: Fix whitespaces at cx24110_set_fec() Mauro Carvalho Chehab
@ 2014-11-05 12:03 ` Mauro Carvalho Chehab
  2014-11-05 12:03 ` [PATCH 5/5] [media] cx24110: Simplify error handling at cx24110_set_fec() Mauro Carvalho Chehab
  3 siblings, 0 replies; 7+ messages in thread
From: Mauro Carvalho Chehab @ 2014-11-05 12:03 UTC (permalink / raw)
  To: Linux Media Mailing List; +Cc: Mauro Carvalho Chehab, Mauro Carvalho Chehab

When a parameter is invalid, the right return code is
-EINVAL.

Signed-off-by: Mauro Carvalho Chehab <mchehab@osg.samsung.com>

diff --git a/drivers/media/dvb-frontends/cx24110.c b/drivers/media/dvb-frontends/cx24110.c
index 4f5c992afe67..5a31b3f59306 100644
--- a/drivers/media/dvb-frontends/cx24110.c
+++ b/drivers/media/dvb-frontends/cx24110.c
@@ -217,8 +217,7 @@ static int cx24110_set_fec (struct cx24110_state* state, fe_code_rate_t fec)
 			cx24110_writereg(state, 0x1b, g2[fec]);
 			/* not sure if this is the right way: I always used AutoAcq mode */
 	   } else
-		   return -EOPNOTSUPP;
-/* fixme (low): which is the correct return code? */
+		   return -EINVAL;
 	}
 	return 0;
 }
-- 
1.9.3


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH 5/5] [media] cx24110: Simplify error handling at cx24110_set_fec()
  2014-11-05 12:03 [PATCH 1/5] [media] cx22700: Fix potential buffer overflow Mauro Carvalho Chehab
                   ` (2 preceding siblings ...)
  2014-11-05 12:03 ` [PATCH 4/5] [media] cx23110: Fix return code for cx24110_set_fec() Mauro Carvalho Chehab
@ 2014-11-05 12:03 ` Mauro Carvalho Chehab
  3 siblings, 0 replies; 7+ messages in thread
From: Mauro Carvalho Chehab @ 2014-11-05 12:03 UTC (permalink / raw)
  To: Linux Media Mailing List; +Cc: Mauro Carvalho Chehab, Mauro Carvalho Chehab

move the return to happen before the logic. This way, we can
avoid one extra identation.

This also fixes an identation issue on this function.

Signed-off-by: Mauro Carvalho Chehab <mchehab@osg.samsung.com>

diff --git a/drivers/media/dvb-frontends/cx24110.c b/drivers/media/dvb-frontends/cx24110.c
index 5a31b3f59306..7b510f2ae20f 100644
--- a/drivers/media/dvb-frontends/cx24110.c
+++ b/drivers/media/dvb-frontends/cx24110.c
@@ -177,10 +177,8 @@ static int cx24110_set_inversion (struct cx24110_state* state, fe_spectral_inver
 	return 0;
 }
 
-static int cx24110_set_fec (struct cx24110_state* state, fe_code_rate_t fec)
+static int cx24110_set_fec(struct cx24110_state* state, fe_code_rate_t fec)
 {
-/* fixme (low): error handling */
-
 	static const int rate[FEC_AUTO] = {-1,    1,    2,    3,    5,    7, -1};
 	static const int g1[FEC_AUTO]   = {-1, 0x01, 0x02, 0x05, 0x15, 0x45, -1};
 	static const int g2[FEC_AUTO]   = {-1, 0x01, 0x03, 0x06, 0x1a, 0x7a, -1};
@@ -208,16 +206,16 @@ static int cx24110_set_fec (struct cx24110_state* state, fe_code_rate_t fec)
 	} else {
 		cx24110_writereg(state, 0x37, cx24110_readreg(state, 0x37) | 0x20);
 		/* set AcqVitDis bit */
-		if (rate[fec] > 0) {
-			cx24110_writereg(state, 0x05, (cx24110_readreg(state, 0x05) & 0xf0) | rate[fec]);
-			/* set nominal Viterbi rate */
-			cx24110_writereg(state, 0x22, (cx24110_readreg(state, 0x22) & 0xf0) | rate[fec]);
-			/* set current Viterbi rate */
-			cx24110_writereg(state, 0x1a, g1[fec]);
-			cx24110_writereg(state, 0x1b, g2[fec]);
-			/* not sure if this is the right way: I always used AutoAcq mode */
-	   } else
-		   return -EINVAL;
+		if (rate[fec] < 0)
+			return -EINVAL;
+
+		cx24110_writereg(state, 0x05, (cx24110_readreg(state, 0x05) & 0xf0) | rate[fec]);
+		/* set nominal Viterbi rate */
+		cx24110_writereg(state, 0x22, (cx24110_readreg(state, 0x22) & 0xf0) | rate[fec]);
+		/* set current Viterbi rate */
+		cx24110_writereg(state, 0x1a, g1[fec]);
+		cx24110_writereg(state, 0x1b, g2[fec]);
+		/* not sure if this is the right way: I always used AutoAcq mode */
 	}
 	return 0;
 }
-- 
1.9.3


^ permalink raw reply related	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2014-11-05 12:41 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-11-05 12:03 [PATCH 1/5] [media] cx22700: Fix potential buffer overflow Mauro Carvalho Chehab
2014-11-05 12:03 ` [PATCH 2/5] [media] cx24110: Fix a spatch warning Mauro Carvalho Chehab
2014-11-05 12:21   ` Hans Verkuil
2014-11-05 12:41     ` Mauro Carvalho Chehab
2014-11-05 12:03 ` [PATCH 3/5] [media] cx24110: Fix whitespaces at cx24110_set_fec() Mauro Carvalho Chehab
2014-11-05 12:03 ` [PATCH 4/5] [media] cx23110: Fix return code for cx24110_set_fec() Mauro Carvalho Chehab
2014-11-05 12:03 ` [PATCH 5/5] [media] cx24110: Simplify error handling at cx24110_set_fec() Mauro Carvalho Chehab

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.