* [patch] tvaudio: remove bogus check
@ 2009-03-02 15:12 Vitaly Wool
2009-03-02 16:02 ` Alexey Klimov
2009-03-02 19:47 ` Mauro Carvalho Chehab
0 siblings, 2 replies; 4+ messages in thread
From: Vitaly Wool @ 2009-03-02 15:12 UTC (permalink / raw)
To: Mauro Carvalho Chehab; +Cc: video4linux-list
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.
drivers/media/video/tvaudio.c | 17 +----------------
1 file changed, 1 insertion(+), 16 deletions(-)
Signed-off-by: Vitaly Wool <vital@embeddedalley.com>
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);
}
--
video4linux-list mailing list
Unsubscribe mailto:video4linux-list-request@redhat.com?subject=unsubscribe
https://www.redhat.com/mailman/listinfo/video4linux-list
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [patch] tvaudio: remove bogus check
2009-03-02 15:12 [patch] tvaudio: remove bogus check Vitaly Wool
@ 2009-03-02 16:02 ` Alexey Klimov
2009-03-02 19:47 ` Mauro Carvalho Chehab
1 sibling, 0 replies; 4+ messages in thread
From: Alexey Klimov @ 2009-03-02 16:02 UTC (permalink / raw)
To: Vitaly Wool; +Cc: video4linux-list, Mauro Carvalho Chehab
Hello, Vitaly
Now the main development maillist of V4L is linux-media@vger.kernel.org.
Posting to that maillist also helps patchwork.kernel.org to handle
patches correctly.
Mauro, what is the status of video4linux-list@redhat.com? Is that user
maillist, or is it deprecated mail that should be disabled ?
On Mon, 2009-03-02 at 18:12 +0300, Vitaly Wool 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.
>
> drivers/media/video/tvaudio.c | 17 +----------------
> 1 file changed, 1 insertion(+), 16 deletions(-)
>
> Signed-off-by: Vitaly Wool <vital@embeddedalley.com>
>
> 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);
> }
>
>
> --
> video4linux-list mailing list
> Unsubscribe mailto:video4linux-list-request@redhat.com?subject=unsubscribe
> https://www.redhat.com/mailman/listinfo/video4linux-list
--
Best regards, Klimov Alexey
--
video4linux-list mailing list
Unsubscribe mailto:video4linux-list-request@redhat.com?subject=unsubscribe
https://www.redhat.com/mailman/listinfo/video4linux-list
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [patch] tvaudio: remove bogus check
2009-03-02 15:12 [patch] tvaudio: remove bogus check Vitaly Wool
2009-03-02 16:02 ` Alexey Klimov
@ 2009-03-02 19:47 ` Mauro Carvalho Chehab
2009-03-03 13:25 ` Vitaly Wool
1 sibling, 1 reply; 4+ messages in thread
From: Mauro Carvalho Chehab @ 2009-03-02 19:47 UTC (permalink / raw)
To: Vitaly Wool; +Cc: video4linux-list
On Mon, 02 Mar 2009 18:12:06 +0300
Vitaly Wool <vital@embeddedalley.com> 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@embeddedalley.com>
>
> 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@redhat.com?subject=unsubscribe
https://www.redhat.com/mailman/listinfo/video4linux-list
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [patch] tvaudio: remove bogus check
2009-03-02 19:47 ` Mauro Carvalho Chehab
@ 2009-03-03 13:25 ` Vitaly Wool
0 siblings, 0 replies; 4+ messages in thread
From: Vitaly Wool @ 2009-03-03 13:25 UTC (permalink / raw)
To: Mauro Carvalho Chehab; +Cc: video4linux-list
Hello Mauro,
Mauro Carvalho Chehab wrote:
> 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.
agreed. Here's the update:
tvaudio.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
Signed-off-by: Vitaly Wool <vital@embeddedalley.com>
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-03 10:35:10.000000000 +0300
@@ -54,7 +54,7 @@
/* ---------------------------------------------------------------------- */
/* our structs */
-#define MAXREGS 64
+#define MAXREGS 256
struct CHIPSTATE;
typedef int (*getvalue)(int);
--
video4linux-list mailing list
Unsubscribe mailto:video4linux-list-request@redhat.com?subject=unsubscribe
https://www.redhat.com/mailman/listinfo/video4linux-list
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2009-03-03 13:33 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2009-03-02 15:12 [patch] tvaudio: remove bogus check Vitaly Wool
2009-03-02 16:02 ` Alexey Klimov
2009-03-02 19:47 ` Mauro Carvalho Chehab
2009-03-03 13:25 ` Vitaly Wool
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox