Re: [patch] tvaudio: remove bogus check

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Mon, 02 Mar 2009 18:12:06 +0300
Vitaly Wool <vital@xxxxxxxxxxxxxxxxx> wrote:

> Hello Mauro,
> 
> below is the patch that removes the subaddr check against ARRAY_SIZE(chip->shadow.bytes).
> Regardless of anything, the 'bytes' array is 64 bytes large so this check disables 
> easy standard programming (TDA9874A_ESP) which has a number of 255 which is hardly the
> intended behavior.
> 
> As a matter of fact, we can think of separate check for this case like
> 	if (subaddr + 1 >= ARRAY_SIZE(chip->shadow.bytes) ||
> 	    subaddr != 0xFF) {
> 		... /* weird register, refuse */
> 	}
> but I'm not sure if there are no other special cases so for now I suggest to just disable
> it.

This patch is wrong, since it will allow the access of an inexistent position at the shadow array:

	chip->shadow.bytes[subaddr+1] = val;

The proper fix is to increase the size of the shadow.bytes array to properly
handle the subaddr = 0xff. Something like:

-#define MAXREGS 64
+#define MAXREGS 256

Except for allocating a few more bytes, such patch won't have any other drawback.

> 
>  drivers/media/video/tvaudio.c |   17 +----------------
>  1 file changed, 1 insertion(+), 16 deletions(-)
> 
> Signed-off-by: Vitaly Wool <vital@xxxxxxxxxxxxxxxxx> 
> 
> Index: linux-next/drivers/media/video/tvaudio.c
> ===================================================================
> --- linux-next.orig/drivers/media/video/tvaudio.c	2009-03-02 17:50:40.000000000 +0300
> +++ linux-next/drivers/media/video/tvaudio.c	2009-03-02 18:08:08.000000000 +0300
> @@ -169,13 +169,6 @@
>  			return -1;
>  		}
>  	} else {
> -		if (subaddr + 1 >= ARRAY_SIZE(chip->shadow.bytes)) {
> -			v4l2_info(sd,
> -				"Tried to access a non-existent register: %d\n",
> -				subaddr);
> -			return -EINVAL;
> -		}
> -
>  		v4l2_dbg(1, debug, sd, "chip_write: reg%d=0x%x\n",
>  			subaddr, val);
>  		chip->shadow.bytes[subaddr+1] = val;
> @@ -198,16 +191,8 @@
>  	if (mask != 0) {
>  		if (subaddr < 0) {
>  			val = (chip->shadow.bytes[1] & ~mask) | (val & mask);
> -		} else {
> -			if (subaddr + 1 >= ARRAY_SIZE(chip->shadow.bytes)) {
> -				v4l2_info(sd,
> -					"Tried to access a non-existent register: %d\n",
> -					subaddr);
> -				return -EINVAL;
> -			}
> -
> +		} else
>  			val = (chip->shadow.bytes[subaddr+1] & ~mask) | (val & mask);
> -		}
>  	}
>  	return chip_write(chip, subaddr, val);
>  }
> 
> 




Cheers,
Mauro

--
video4linux-list mailing list
Unsubscribe mailto:video4linux-list-request@xxxxxxxxxx?subject=unsubscribe
https://www.redhat.com/mailman/listinfo/video4linux-list

[Index of Archives]     [Linux Media]     [Older V4L]     [Linux DVB]     [Video Disk Recorder]     [Linux Kernel]     [Asterisk]     [DCCP]     [Netdev]     [X.org]     [Util Linux]     [Xfree86]     [Free Photo Albums]     [Fedora Users]     [Fedora Women]     [ALSA Users]     [ALSA Devel]     [SSH]     [DVB Maintainers]     [Linux USB]     [Yosemite Information]
  Powered by Linux