[PATCH] media: dib8000: validate SNR exponent before shifting

[PATCH] media: dib8000: validate SNR exponent before shifting

[Ilya Krutskih]

Decode the SNR exponent as a signed value instead of storing it in a
u32. Subtracting 0x40 from the unsigned exponent currently causes it
to wrap around when the encoded value represents a negative number.

Keep the signal and noise mantissas unsigned, as they are used in
unsigned fixed-point arithmetic. Check exponent values before
shifting by check_shl_overflow().

Return the existing invalid SNR value when the exponent cannot be
represented safely.

Fixes: 77e2c0f5d471 ("V4L/DVB (12900): DiB8000: added support for DiBcom ISDB-T/ISDB-Tsb demodulator DiB8000")
Cc: stable@vger.kernel.org
Signed-off-by: Ilya Krutskih <devsec@tpz.ru>
---
 drivers/media/dvb-frontends/dib8000.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/media/dvb-frontends/dib8000.c b/drivers/media/dvb-frontends/dib8000.c
index d291113291e0..442ec06e34dc 100644
--- a/drivers/media/dvb-frontends/dib8000.c
+++ b/drivers/media/dvb-frontends/dib8000.c
@@ -11,6 +11,7 @@
 #include <linux/slab.h>
 #include <linux/i2c.h>
 #include <linux/mutex.h>
+#include <linux/overflow.h>
 #include <asm/div64.h>
 
 #include <linux/int_log.h>
@@ -3873,7 +3874,8 @@ static u32 dib8000_get_snr(struct dvb_frontend *fe)
 	exp = (val & 0x3f);
 	if ((exp & 0x20) != 0)
 		exp -= 0x40;
-	n <<= exp+16;
+	if (check_shl_overflow(n, exp + 16, &n))
+		return 0xffffffff;
 
 	if (state->revision != 0x8090)
 		val = dib8000_read_word(state, 543);
@@ -3883,7 +3885,8 @@ static u32 dib8000_get_snr(struct dvb_frontend *fe)
 	exp = (val & 0x3f);
 	if ((exp & 0x20) != 0)
 		exp -= 0x40;
-	s <<= exp+16;
+	if (check_shl_overflow(s, exp + 16, &s))
+		return 0xffffffff;
 
 	if (n > 0) {
 		u32 t = (s/n) << 16;
-- 
2.43.0

Re: [PATCH] media: dib8000: validate SNR exponent before shifting

[Sergey Shtylyov]

On 10/22/26 8:23 PM, Ilya Krutskih wrote:

   Please consider using --no-git-fallback with scripts/get_maintainer.pl --
I don't see what do I have to do with this patch (other than fixing another
bug in this driver earlier).

> Decode the SNR exponent as a signed value instead of storing it in a
> u32.

   I don't see where are you changing the type from u32 in this version of
patch -- this phrase seems to be copied from the 1st (internal) version...

> Subtracting 0x40 from the unsigned exponent currently causes it
> to wrap around when the encoded value represents a negative number.

   Seems to be the intended effect...

> Keep the signal and noise mantissas unsigned, as they are used in
> unsigned fixed-point arithmetic.

   Again, this phrase seems to be (mindlessly?) copied from the 1st
version... :-)

> Check exponent values before shifting by check_shl_overflow().

   Yes, that's what the patch below actually does! :-)

> Return the existing invalid SNR value when the exponent cannot be
> represented safely.
> 
> Fixes: 77e2c0f5d471 ("V4L/DVB (12900): DiB8000: added support for DiBcom ISDB-T/ISDB-Tsb demodulator DiB8000")
> Cc: stable@vger.kernel.org
> Signed-off-by: Ilya Krutskih <devsec@tpz.ru>
> ---
>  drivers/media/dvb-frontends/dib8000.c | 7 +++++--
>  1 file changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/media/dvb-frontends/dib8000.c b/drivers/media/dvb-frontends/dib8000.c
> index d291113291e0..442ec06e34dc 100644
> --- a/drivers/media/dvb-frontends/dib8000.c
> +++ b/drivers/media/dvb-frontends/dib8000.c
[...]> @@ -3873,7 +3874,8 @@ static u32 dib8000_get_snr(struct dvb_frontend *fe)
>         exp = (val & 0x3f);
>         if ((exp & 0x20) != 0)
>                 exp -= 0x40;
> -       n <<= exp+16;
> +       if (check_shl_overflow(n, exp + 16, &n))

   The question is whether the hardware can actually return exp < -16 or > 15...

> +               return 0xffffffff;
> 
>         if (state->revision != 0x8090)
>                 val = dib8000_read_word(state, 543);
> @@ -3883,7 +3885,8 @@ static u32 dib8000_get_snr(struct dvb_frontend *fe)
>         exp = (val & 0x3f);
>         if ((exp & 0x20) != 0)
>                 exp -= 0x40;
> -       s <<= exp+16;
> +       if (check_shl_overflow(s, exp + 16, &s))
> +               return 0xffffffff;
> 
>         if (n > 0) {
>                 u32 t = (s/n) << 16;[...]

MBR, Sergey