linux-media.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] media: ttusb-dec: fix heap-buffer-overflow in ttusb_dec_process_urb_frame()
@ 2025-12-22  0:20 pip-izony
  2025-12-22  5:46 ` [PATCH v2] " pip-izony
  0 siblings, 1 reply; 6+ messages in thread
From: pip-izony @ 2025-12-22  0:20 UTC (permalink / raw)
  To: Mauro Carvalho Chehab
  Cc: Seungjin Bae, Kyungtae Kim, Sanghoon Choi, linux-media,
	linux-kernel

From: Seungjin Bae <eeodqql09@gmail.com>

The `ttusb_dec_process_urb_frame()` parses the PVA packet from the
USB device. However, it doesn't check whether the calculated
`packet_payload_length` exceeds the size of the `packet` buffer.

The `packet` buffer has a fixed size of `MAX_PVA_LENGTH + 4`. However,
`packet_payload_length` is derived from 2 bytes of the input data,
allowing a maximum value of 65543 bytes (8 + 0xFFFF).

If a malicious USB device sends a packet with crafted data, it
triggers a heap buffer overflow. This allows an attacker to overwrite
adjacent fields in the `struct ttusb_dec`. Specifically, the `a_pes2ts`
field, which contains a callback function pointer, is located after the
`packet` buffer. Overwriting this pointer can lead to control flow
hijacking.

Fix this by adding a bounds check for the parsed length against the
buffer size.

Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Co-developed-by: Sanghoon Choi <csh0052@gmail.com>
Signed-off-by: Sanghoon Choi <csh0052@gmail.com>
Signed-off-by: Seungjin Bae <eeodqql09@gmail.com>
---
 drivers/media/usb/ttusb-dec/ttusb_dec.c | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/drivers/media/usb/ttusb-dec/ttusb_dec.c b/drivers/media/usb/ttusb-dec/ttusb_dec.c
index b4575fe89c95..77452ff2522f 100644
--- a/drivers/media/usb/ttusb-dec/ttusb_dec.c
+++ b/drivers/media/usb/ttusb-dec/ttusb_dec.c
@@ -703,10 +703,18 @@ static void ttusb_dec_process_urb_frame(struct ttusb_dec *dec, u8 *b,
 
 			if (dec->packet_type == TTUSB_DEC_PACKET_PVA &&
 			    dec->packet_length == 8) {
-				dec->packet_state++;
-				dec->packet_payload_length = 8 +
+				int len = 8 +
 					(dec->packet[6] << 8) +
 					dec->packet[7];
+
+				if (len > MAX_PVA_LENGTH + 4) {
+					printk("%s: packet too long - discarding\n",
+					       __func__);
+					dec->packet_state = 0;
+				} else {
+					dec->packet_state++;
+					dec->packet_payload_length = len;
+				}
 			} else if (dec->packet_type ==
 					TTUSB_DEC_PACKET_SECTION &&
 				   dec->packet_length == 5) {
-- 
2.43.0


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

* [PATCH v2] media: ttusb-dec: fix heap-buffer-overflow in ttusb_dec_process_urb_frame()
  2025-12-22  0:20 [PATCH] media: ttusb-dec: fix heap-buffer-overflow in ttusb_dec_process_urb_frame() pip-izony
@ 2025-12-22  5:46 ` pip-izony
  2025-12-23  0:31   ` kernel test robot
                     ` (2 more replies)
  0 siblings, 3 replies; 6+ messages in thread
From: pip-izony @ 2025-12-22  5:46 UTC (permalink / raw)
  To: Mauro Carvalho Chehab
  Cc: Seungjin Bae, Kyungtae Kim, Sanghoon Choi, linux-media,
	linux-kernel

From: Seungjin Bae <eeodqql09@gmail.com>

The `ttusb_dec_process_urb_frame()` parses the PVA packet from the
USB device. However, it doesn't check whether the calculated
`packet_payload_length` exceeds the size of the `packet` buffer.

The `packet` buffer has a fixed size of `MAX_PVA_LENGTH + 4`. However,
`packet_payload_length` is derived from 2 bytes of the input data,
allowing a maximum value of 65543 bytes (8 + 0xFFFF).

If a malicious USB device sends a packet with crafted data, it
triggers a heap buffer overflow. This allows an attacker to overwrite
adjacent fields in the `struct ttusb_dec`. Specifically, the `a_pes2ts`
field, which contains a callback function pointer, is located after the
`packet` buffer. Overwriting this pointer can lead to control flow
hijacking.

Fix this by adding a bounds check for the parsed length against the
buffer size.

Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Co-developed-by: Sanghoon Choi <csh0052@gmail.com>
Signed-off-by: Sanghoon Choi <csh0052@gmail.com>
Signed-off-by: Seungjin Bae <eeodqql09@gmail.com>
---
 v1 -> v2: Change warning function
 
 drivers/media/usb/ttusb-dec/ttusb_dec.c | 13 +++++++++++--
 1 file changed, 11 insertions(+), 2 deletions(-)

diff --git a/drivers/media/usb/ttusb-dec/ttusb_dec.c b/drivers/media/usb/ttusb-dec/ttusb_dec.c
index b4575fe89c95..0e983783e787 100644
--- a/drivers/media/usb/ttusb-dec/ttusb_dec.c
+++ b/drivers/media/usb/ttusb-dec/ttusb_dec.c
@@ -703,10 +703,19 @@ static void ttusb_dec_process_urb_frame(struct ttusb_dec *dec, u8 *b,
 
 			if (dec->packet_type == TTUSB_DEC_PACKET_PVA &&
 			    dec->packet_length == 8) {
-				dec->packet_state++;
-				dec->packet_payload_length = 8 +
+				int len = 8 +
 					(dec->packet[6] << 8) +
 					dec->packet[7];
+
+				if (len > MAX_PVA_LENGTH + 4) {
+					dev_warn(&dec->udev->dev,
+						"%s: packet too long - discarding\n"
+						__func__);
+					dec->packet_state = 0;
+				} else {
+					dec->packet_state++;
+					dec->packet_payload_length = len;
+				}
 			} else if (dec->packet_type ==
 					TTUSB_DEC_PACKET_SECTION &&
 				   dec->packet_length == 5) {
-- 
2.43.0


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

* Re: [PATCH v2] media: ttusb-dec: fix heap-buffer-overflow in ttusb_dec_process_urb_frame()
  2025-12-22  5:46 ` [PATCH v2] " pip-izony
@ 2025-12-23  0:31   ` kernel test robot
  2025-12-23  1:01   ` [PATCH v3] " pip-izony
  2025-12-23  1:17   ` [PATCH v2] " kernel test robot
  2 siblings, 0 replies; 6+ messages in thread
From: kernel test robot @ 2025-12-23  0:31 UTC (permalink / raw)
  To: pip-izony, Mauro Carvalho Chehab
  Cc: oe-kbuild-all, linux-media, Seungjin Bae, Kyungtae Kim,
	Sanghoon Choi, linux-kernel

Hi pip-izony,

kernel test robot noticed the following build errors:

[auto build test ERROR on linuxtv-media-pending/master]
[also build test ERROR on media-tree/master sailus-media-tree/master linus/master sailus-media-tree/streams v6.19-rc2 next-20251219]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/pip-izony/media-ttusb-dec-fix-heap-buffer-overflow-in-ttusb_dec_process_urb_frame/20251222-134809
base:   https://git.linuxtv.org/media-ci/media-pending.git master
patch link:    https://lore.kernel.org/r/20251222054644.938208-2-eeodqql09%40gmail.com
patch subject: [PATCH v2] media: ttusb-dec: fix heap-buffer-overflow in ttusb_dec_process_urb_frame()
config: openrisc-allmodconfig (https://download.01.org/0day-ci/archive/20251223/202512230853.nWSWHk5e-lkp@intel.com/config)
compiler: or1k-linux-gcc (GCC) 15.1.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20251223/202512230853.nWSWHk5e-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202512230853.nWSWHk5e-lkp@intel.com/

All error/warnings (new ones prefixed by >>):

   In file included from include/asm-generic/bug.h:22,
                    from arch/openrisc/include/asm/bug.h:5,
                    from include/linux/bug.h:5,
                    from include/linux/thread_info.h:13,
                    from include/asm-generic/preempt.h:5,
                    from ./arch/openrisc/include/generated/asm/preempt.h:1,
                    from include/linux/preempt.h:79,
                    from include/linux/spinlock.h:56,
                    from include/linux/mmzone.h:8,
                    from include/linux/gfp.h:7,
                    from include/linux/umh.h:4,
                    from include/linux/kmod.h:9,
                    from include/linux/module.h:18,
                    from drivers/media/usb/ttusb-dec/ttusb_dec.c:10:
   drivers/media/usb/ttusb-dec/ttusb_dec.c: In function 'ttusb_dec_process_urb_frame':
>> drivers/media/usb/ttusb-dec/ttusb_dec.c:713:49: error: expected ')' before '__func__'
     713 |                                                 __func__);
         |                                                 ^~~~~~~~
   include/linux/printk.h:436:42: note: in definition of macro '__printk_index_emit'
     436 |                 if (__builtin_constant_p(_fmt) && __builtin_constant_p(_level)) { \
         |                                          ^~~~
   include/linux/dev_printk.h:105:9: note: in expansion of macro 'printk_index_subsys_emit'
     105 |         printk_index_subsys_emit("%s %s: ", level, fmt)
         |         ^~~~~~~~~~~~~~~~~~~~~~~~
   include/linux/dev_printk.h:109:17: note: in expansion of macro 'dev_printk_index_emit'
     109 |                 dev_printk_index_emit(level, fmt);                      \
         |                 ^~~~~~~~~~~~~~~~~~~~~
   include/linux/dev_printk.h:156:9: note: in expansion of macro 'dev_printk_index_wrap'
     156 |         dev_printk_index_wrap(_dev_warn, KERN_WARNING, dev, dev_fmt(fmt), ##__VA_ARGS__)
         |         ^~~~~~~~~~~~~~~~~~~~~
   include/linux/dev_printk.h:156:61: note: in expansion of macro 'dev_fmt'
     156 |         dev_printk_index_wrap(_dev_warn, KERN_WARNING, dev, dev_fmt(fmt), ##__VA_ARGS__)
         |                                                             ^~~~~~~
   drivers/media/usb/ttusb-dec/ttusb_dec.c:711:41: note: in expansion of macro 'dev_warn'
     711 |                                         dev_warn(&dec->udev->dev,
         |                                         ^~~~~~~~
   include/linux/printk.h:436:41: note: to match this '('
     436 |                 if (__builtin_constant_p(_fmt) && __builtin_constant_p(_level)) { \
         |                                         ^
   include/linux/printk.h:479:9: note: in expansion of macro '__printk_index_emit'
     479 |         __printk_index_emit(fmt, level, subsys_fmt_prefix)
         |         ^~~~~~~~~~~~~~~~~~~
   include/linux/dev_printk.h:105:9: note: in expansion of macro 'printk_index_subsys_emit'
     105 |         printk_index_subsys_emit("%s %s: ", level, fmt)
         |         ^~~~~~~~~~~~~~~~~~~~~~~~
   include/linux/dev_printk.h:109:17: note: in expansion of macro 'dev_printk_index_emit'
     109 |                 dev_printk_index_emit(level, fmt);                      \
         |                 ^~~~~~~~~~~~~~~~~~~~~
   include/linux/dev_printk.h:156:9: note: in expansion of macro 'dev_printk_index_wrap'
     156 |         dev_printk_index_wrap(_dev_warn, KERN_WARNING, dev, dev_fmt(fmt), ##__VA_ARGS__)
         |         ^~~~~~~~~~~~~~~~~~~~~
   drivers/media/usb/ttusb-dec/ttusb_dec.c:711:41: note: in expansion of macro 'dev_warn'
     711 |                                         dev_warn(&dec->udev->dev,
         |                                         ^~~~~~~~
>> drivers/media/usb/ttusb-dec/ttusb_dec.c:713:49: error: expected ')' before '__func__'
     713 |                                                 __func__);
         |                                                 ^~~~~~~~
   include/linux/printk.h:445:61: note: in definition of macro '__printk_index_emit'
     445 |                                 .fmt = __builtin_constant_p(_fmt) ? (_fmt) : NULL, \
         |                                                             ^~~~
   include/linux/dev_printk.h:105:9: note: in expansion of macro 'printk_index_subsys_emit'
     105 |         printk_index_subsys_emit("%s %s: ", level, fmt)
         |         ^~~~~~~~~~~~~~~~~~~~~~~~
   include/linux/dev_printk.h:109:17: note: in expansion of macro 'dev_printk_index_emit'
     109 |                 dev_printk_index_emit(level, fmt);                      \
         |                 ^~~~~~~~~~~~~~~~~~~~~
   include/linux/dev_printk.h:156:9: note: in expansion of macro 'dev_printk_index_wrap'
     156 |         dev_printk_index_wrap(_dev_warn, KERN_WARNING, dev, dev_fmt(fmt), ##__VA_ARGS__)
         |         ^~~~~~~~~~~~~~~~~~~~~
   include/linux/dev_printk.h:156:61: note: in expansion of macro 'dev_fmt'
     156 |         dev_printk_index_wrap(_dev_warn, KERN_WARNING, dev, dev_fmt(fmt), ##__VA_ARGS__)
         |                                                             ^~~~~~~
   drivers/media/usb/ttusb-dec/ttusb_dec.c:711:41: note: in expansion of macro 'dev_warn'
     711 |                                         dev_warn(&dec->udev->dev,
         |                                         ^~~~~~~~
   include/linux/printk.h:445:60: note: to match this '('
     445 |                                 .fmt = __builtin_constant_p(_fmt) ? (_fmt) : NULL, \
         |                                                            ^
   include/linux/printk.h:479:9: note: in expansion of macro '__printk_index_emit'
     479 |         __printk_index_emit(fmt, level, subsys_fmt_prefix)
         |         ^~~~~~~~~~~~~~~~~~~
   include/linux/dev_printk.h:105:9: note: in expansion of macro 'printk_index_subsys_emit'
     105 |         printk_index_subsys_emit("%s %s: ", level, fmt)
         |         ^~~~~~~~~~~~~~~~~~~~~~~~
   include/linux/dev_printk.h:109:17: note: in expansion of macro 'dev_printk_index_emit'
     109 |                 dev_printk_index_emit(level, fmt);                      \
         |                 ^~~~~~~~~~~~~~~~~~~~~
   include/linux/dev_printk.h:156:9: note: in expansion of macro 'dev_printk_index_wrap'
     156 |         dev_printk_index_wrap(_dev_warn, KERN_WARNING, dev, dev_fmt(fmt), ##__VA_ARGS__)
         |         ^~~~~~~~~~~~~~~~~~~~~
   drivers/media/usb/ttusb-dec/ttusb_dec.c:711:41: note: in expansion of macro 'dev_warn'
     711 |                                         dev_warn(&dec->udev->dev,
         |                                         ^~~~~~~~
>> drivers/media/usb/ttusb-dec/ttusb_dec.c:713:49: error: expected ')' before '__func__'
     713 |                                                 __func__);
         |                                                 ^~~~~~~~
   include/linux/printk.h:445:70: note: in definition of macro '__printk_index_emit'
     445 |                                 .fmt = __builtin_constant_p(_fmt) ? (_fmt) : NULL, \
         |                                                                      ^~~~
   include/linux/dev_printk.h:105:9: note: in expansion of macro 'printk_index_subsys_emit'
     105 |         printk_index_subsys_emit("%s %s: ", level, fmt)
         |         ^~~~~~~~~~~~~~~~~~~~~~~~
   include/linux/dev_printk.h:109:17: note: in expansion of macro 'dev_printk_index_emit'
     109 |                 dev_printk_index_emit(level, fmt);                      \
         |                 ^~~~~~~~~~~~~~~~~~~~~
   include/linux/dev_printk.h:156:9: note: in expansion of macro 'dev_printk_index_wrap'
     156 |         dev_printk_index_wrap(_dev_warn, KERN_WARNING, dev, dev_fmt(fmt), ##__VA_ARGS__)
         |         ^~~~~~~~~~~~~~~~~~~~~
   include/linux/dev_printk.h:156:61: note: in expansion of macro 'dev_fmt'
     156 |         dev_printk_index_wrap(_dev_warn, KERN_WARNING, dev, dev_fmt(fmt), ##__VA_ARGS__)
         |                                                             ^~~~~~~
   drivers/media/usb/ttusb-dec/ttusb_dec.c:711:41: note: in expansion of macro 'dev_warn'
     711 |                                         dev_warn(&dec->udev->dev,
         |                                         ^~~~~~~~
   include/linux/printk.h:445:69: note: to match this '('
     445 |                                 .fmt = __builtin_constant_p(_fmt) ? (_fmt) : NULL, \
         |                                                                     ^
   include/linux/printk.h:479:9: note: in expansion of macro '__printk_index_emit'
     479 |         __printk_index_emit(fmt, level, subsys_fmt_prefix)
         |         ^~~~~~~~~~~~~~~~~~~
   include/linux/dev_printk.h:105:9: note: in expansion of macro 'printk_index_subsys_emit'
     105 |         printk_index_subsys_emit("%s %s: ", level, fmt)
         |         ^~~~~~~~~~~~~~~~~~~~~~~~
   include/linux/dev_printk.h:109:17: note: in expansion of macro 'dev_printk_index_emit'
     109 |                 dev_printk_index_emit(level, fmt);                      \
         |                 ^~~~~~~~~~~~~~~~~~~~~
   include/linux/dev_printk.h:156:9: note: in expansion of macro 'dev_printk_index_wrap'
     156 |         dev_printk_index_wrap(_dev_warn, KERN_WARNING, dev, dev_fmt(fmt), ##__VA_ARGS__)
         |         ^~~~~~~~~~~~~~~~~~~~~
   drivers/media/usb/ttusb-dec/ttusb_dec.c:711:41: note: in expansion of macro 'dev_warn'
     711 |                                         dev_warn(&dec->udev->dev,
         |                                         ^~~~~~~~
   In file included from include/linux/device.h:15,
                    from include/linux/pci.h:37,
                    from drivers/media/usb/ttusb-dec/ttusb_dec.c:11:
>> drivers/media/usb/ttusb-dec/ttusb_dec.c:713:49: error: expected ')' before '__func__'
     713 |                                                 __func__);
         |                                                 ^~~~~~~~
   include/linux/dev_printk.h:110:30: note: in definition of macro 'dev_printk_index_wrap'
     110 |                 _p_func(dev, fmt, ##__VA_ARGS__);                       \
         |                              ^~~
   include/linux/dev_printk.h:156:61: note: in expansion of macro 'dev_fmt'
     156 |         dev_printk_index_wrap(_dev_warn, KERN_WARNING, dev, dev_fmt(fmt), ##__VA_ARGS__)
         |                                                             ^~~~~~~
   drivers/media/usb/ttusb-dec/ttusb_dec.c:711:41: note: in expansion of macro 'dev_warn'
     711 |                                         dev_warn(&dec->udev->dev,
         |                                         ^~~~~~~~
   include/linux/dev_printk.h:110:24: note: to match this '('
     110 |                 _p_func(dev, fmt, ##__VA_ARGS__);                       \
         |                        ^
   include/linux/dev_printk.h:156:9: note: in expansion of macro 'dev_printk_index_wrap'
     156 |         dev_printk_index_wrap(_dev_warn, KERN_WARNING, dev, dev_fmt(fmt), ##__VA_ARGS__)
         |         ^~~~~~~~~~~~~~~~~~~~~
   drivers/media/usb/ttusb-dec/ttusb_dec.c:711:41: note: in expansion of macro 'dev_warn'
     711 |                                         dev_warn(&dec->udev->dev,
         |                                         ^~~~~~~~
>> drivers/media/usb/ttusb-dec/ttusb_dec.c:712:49: warning: format '%s' expects a matching 'char *' argument [-Wformat=]
     712 |                                                 "%s: packet too long - discarding\n"
         |                                                 ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   include/linux/dev_printk.h:110:30: note: in definition of macro 'dev_printk_index_wrap'
     110 |                 _p_func(dev, fmt, ##__VA_ARGS__);                       \
         |                              ^~~
   include/linux/dev_printk.h:156:61: note: in expansion of macro 'dev_fmt'
     156 |         dev_printk_index_wrap(_dev_warn, KERN_WARNING, dev, dev_fmt(fmt), ##__VA_ARGS__)
         |                                                             ^~~~~~~
   drivers/media/usb/ttusb-dec/ttusb_dec.c:711:41: note: in expansion of macro 'dev_warn'
     711 |                                         dev_warn(&dec->udev->dev,
         |                                         ^~~~~~~~
   drivers/media/usb/ttusb-dec/ttusb_dec.c:712:51: note: format string is defined here
     712 |                                                 "%s: packet too long - discarding\n"
         |                                                  ~^
         |                                                   |
         |                                                   char *


vim +713 drivers/media/usb/ttusb-dec/ttusb_dec.c

   640	
   641	static void ttusb_dec_process_urb_frame(struct ttusb_dec *dec, u8 *b,
   642						int length)
   643	{
   644		swap_bytes(b, length);
   645	
   646		while (length) {
   647			switch (dec->packet_state) {
   648	
   649			case 0:
   650			case 1:
   651			case 2:
   652				if (*b++ == 0xaa)
   653					dec->packet_state++;
   654				else
   655					dec->packet_state = 0;
   656	
   657				length--;
   658				break;
   659	
   660			case 3:
   661				if (*b == 0x00) {
   662					dec->packet_state++;
   663					dec->packet_length = 0;
   664				} else if (*b != 0xaa) {
   665					dec->packet_state = 0;
   666				}
   667	
   668				b++;
   669				length--;
   670				break;
   671	
   672			case 4:
   673				dec->packet[dec->packet_length++] = *b++;
   674	
   675				if (dec->packet_length == 2) {
   676					if (dec->packet[0] == 'A' &&
   677					    dec->packet[1] == 'V') {
   678						dec->packet_type =
   679							TTUSB_DEC_PACKET_PVA;
   680						dec->packet_state++;
   681					} else if (dec->packet[0] == 'S') {
   682						dec->packet_type =
   683							TTUSB_DEC_PACKET_SECTION;
   684						dec->packet_state++;
   685					} else if (dec->packet[0] == 0x00) {
   686						dec->packet_type =
   687							TTUSB_DEC_PACKET_EMPTY;
   688						dec->packet_payload_length = 2;
   689						dec->packet_state = 7;
   690					} else {
   691						printk("%s: unknown packet type: %02x%02x\n",
   692						       __func__,
   693						       dec->packet[0], dec->packet[1]);
   694						dec->packet_state = 0;
   695					}
   696				}
   697	
   698				length--;
   699				break;
   700	
   701			case 5:
   702				dec->packet[dec->packet_length++] = *b++;
   703	
   704				if (dec->packet_type == TTUSB_DEC_PACKET_PVA &&
   705				    dec->packet_length == 8) {
   706					int len = 8 +
   707						(dec->packet[6] << 8) +
   708						dec->packet[7];
   709	
   710					if (len > MAX_PVA_LENGTH + 4) {
   711						dev_warn(&dec->udev->dev,
 > 712							"%s: packet too long - discarding\n"
 > 713							__func__);
   714						dec->packet_state = 0;
   715					} else {
   716						dec->packet_state++;
   717						dec->packet_payload_length = len;
   718					}
   719				} else if (dec->packet_type ==
   720						TTUSB_DEC_PACKET_SECTION &&
   721					   dec->packet_length == 5) {
   722					dec->packet_state++;
   723					dec->packet_payload_length = 5 +
   724						((dec->packet[3] & 0x0f) << 8) +
   725						dec->packet[4];
   726				}
   727	
   728				length--;
   729				break;
   730	
   731			case 6: {
   732				int remainder = dec->packet_payload_length -
   733						dec->packet_length;
   734	
   735				if (length >= remainder) {
   736					memcpy(dec->packet + dec->packet_length,
   737					       b, remainder);
   738					dec->packet_length += remainder;
   739					b += remainder;
   740					length -= remainder;
   741					dec->packet_state++;
   742				} else {
   743					memcpy(&dec->packet[dec->packet_length],
   744					       b, length);
   745					dec->packet_length += length;
   746					length = 0;
   747				}
   748	
   749				break;
   750			}
   751	
   752			case 7: {
   753				int tail = 4;
   754	
   755				dec->packet[dec->packet_length++] = *b++;
   756	
   757				if (dec->packet_type == TTUSB_DEC_PACKET_SECTION &&
   758				    dec->packet_payload_length % 2)
   759					tail++;
   760	
   761				if (dec->packet_length ==
   762				    dec->packet_payload_length + tail) {
   763					ttusb_dec_process_packet(dec);
   764					dec->packet_state = 0;
   765				}
   766	
   767				length--;
   768				break;
   769			}
   770	
   771			default:
   772				printk("%s: illegal packet state encountered.\n",
   773				       __func__);
   774				dec->packet_state = 0;
   775			}
   776		}
   777	}
   778	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

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

* [PATCH v3] media: ttusb-dec: fix heap-buffer-overflow in ttusb_dec_process_urb_frame()
  2025-12-22  5:46 ` [PATCH v2] " pip-izony
  2025-12-23  0:31   ` kernel test robot
@ 2025-12-23  1:01   ` pip-izony
  2025-12-30 19:50     ` [PATCH v4] " pip-izony
  2025-12-23  1:17   ` [PATCH v2] " kernel test robot
  2 siblings, 1 reply; 6+ messages in thread
From: pip-izony @ 2025-12-23  1:01 UTC (permalink / raw)
  To: Mauro Carvalho Chehab
  Cc: Seungjin Bae, Kyungtae Kim, Sanghoon Choi, linux-media,
	linux-kernel

From: Seungjin Bae <eeodqql09@gmail.com>

The `ttusb_dec_process_urb_frame()` parses the PVA packet from the
USB device. However, it doesn't check whether the calculated
`packet_payload_length` exceeds the size of the `packet` buffer.

The `packet` buffer has a fixed size of `MAX_PVA_LENGTH + 4`. However,
`packet_payload_length` is derived from 2 bytes of the input data,
allowing a maximum value of 65543 bytes (8 + 0xFFFF).

If a malicious USB device sends a packet with crafted data, it
triggers a heap buffer overflow. This allows an attacker to overwrite
adjacent fields in the `struct ttusb_dec`. Specifically, the `a_pes2ts`
field, which contains a callback function pointer, is located after the
`packet` buffer. Overwriting this pointer can lead to control flow
hijacking.

Fix this by adding a bounds check for the parsed length against the
buffer size.

Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Co-developed-by: Sanghoon Choi <csh0052@gmail.com>
Signed-off-by: Sanghoon Choi <csh0052@gmail.com>
Signed-off-by: Seungjin Bae <eeodqql09@gmail.com>
---
 v1 -> v2: Change warning function
 v2 -> v3: Add missing comma in the dev_warn argument
 
 drivers/media/usb/ttusb-dec/ttusb_dec.c | 13 +++++++++++--
 1 file changed, 11 insertions(+), 2 deletions(-)

diff --git a/drivers/media/usb/ttusb-dec/ttusb_dec.c b/drivers/media/usb/ttusb-dec/ttusb_dec.c
index b4575fe89c95..0e983783e787 100644
--- a/drivers/media/usb/ttusb-dec/ttusb_dec.c
+++ b/drivers/media/usb/ttusb-dec/ttusb_dec.c
@@ -703,10 +703,19 @@ static void ttusb_dec_process_urb_frame(struct ttusb_dec *dec, u8 *b,
 
 			if (dec->packet_type == TTUSB_DEC_PACKET_PVA &&
 			    dec->packet_length == 8) {
-				dec->packet_state++;
-				dec->packet_payload_length = 8 +
+				int len = 8 +
 					(dec->packet[6] << 8) +
 					dec->packet[7];
+
+				if (len > MAX_PVA_LENGTH + 4) {
+					dev_warn(&dec->udev->dev,
+						"%s: packet too long - discarding\n",
+						__func__);
+					dec->packet_state = 0;
+				} else {
+					dec->packet_state++;
+					dec->packet_payload_length = len;
+				}
 			} else if (dec->packet_type ==
 					TTUSB_DEC_PACKET_SECTION &&
 				   dec->packet_length == 5) {
-- 
2.43.0


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

* Re: [PATCH v2] media: ttusb-dec: fix heap-buffer-overflow in ttusb_dec_process_urb_frame()
  2025-12-22  5:46 ` [PATCH v2] " pip-izony
  2025-12-23  0:31   ` kernel test robot
  2025-12-23  1:01   ` [PATCH v3] " pip-izony
@ 2025-12-23  1:17   ` kernel test robot
  2 siblings, 0 replies; 6+ messages in thread
From: kernel test robot @ 2025-12-23  1:17 UTC (permalink / raw)
  To: pip-izony, Mauro Carvalho Chehab
  Cc: llvm, oe-kbuild-all, linux-media, Seungjin Bae, Kyungtae Kim,
	Sanghoon Choi, linux-kernel

Hi pip-izony,

kernel test robot noticed the following build errors:

[auto build test ERROR on linuxtv-media-pending/master]
[also build test ERROR on media-tree/master sailus-media-tree/master linus/master v6.19-rc2 next-20251219]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/pip-izony/media-ttusb-dec-fix-heap-buffer-overflow-in-ttusb_dec_process_urb_frame/20251222-134809
base:   https://git.linuxtv.org/media-ci/media-pending.git master
patch link:    https://lore.kernel.org/r/20251222054644.938208-2-eeodqql09%40gmail.com
patch subject: [PATCH v2] media: ttusb-dec: fix heap-buffer-overflow in ttusb_dec_process_urb_frame()
config: sparc64-allmodconfig (https://download.01.org/0day-ci/archive/20251223/202512230947.zdHvFcAE-lkp@intel.com/config)
compiler: clang version 22.0.0git (https://github.com/llvm/llvm-project 185f5fd5ce4c65116ca8cf6df467a682ef090499)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20251223/202512230947.zdHvFcAE-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202512230947.zdHvFcAE-lkp@intel.com/

All errors (new ones prefixed by >>):

>> drivers/media/usb/ttusb-dec/ttusb_dec.c:713:7: error: expected ')'
     713 |                                                 __func__);
         |                                                 ^
   drivers/media/usb/ttusb-dec/ttusb_dec.c:711:6: note: to match this '('
     711 |                                         dev_warn(&dec->udev->dev,
         |                                         ^
   include/linux/dev_printk.h:156:2: note: expanded from macro 'dev_warn'
     156 |         dev_printk_index_wrap(_dev_warn, KERN_WARNING, dev, dev_fmt(fmt), ##__VA_ARGS__)
         |         ^
   include/linux/dev_printk.h:109:3: note: expanded from macro 'dev_printk_index_wrap'
     109 |                 dev_printk_index_emit(level, fmt);                      \
         |                 ^
   include/linux/dev_printk.h:105:2: note: expanded from macro 'dev_printk_index_emit'
     105 |         printk_index_subsys_emit("%s %s: ", level, fmt)
         |         ^
   include/linux/printk.h:479:2: note: expanded from macro 'printk_index_subsys_emit'
     479 |         __printk_index_emit(fmt, level, subsys_fmt_prefix)
         |         ^
   include/linux/printk.h:436:27: note: expanded from macro '__printk_index_emit'
     436 |                 if (__builtin_constant_p(_fmt) && __builtin_constant_p(_level)) { \
         |                                         ^
>> drivers/media/usb/ttusb-dec/ttusb_dec.c:713:7: error: expected ')'
     713 |                                                 __func__);
         |                                                 ^
   drivers/media/usb/ttusb-dec/ttusb_dec.c:711:6: note: to match this '('
     711 |                                         dev_warn(&dec->udev->dev,
         |                                         ^
   include/linux/dev_printk.h:156:2: note: expanded from macro 'dev_warn'
     156 |         dev_printk_index_wrap(_dev_warn, KERN_WARNING, dev, dev_fmt(fmt), ##__VA_ARGS__)
         |         ^
   include/linux/dev_printk.h:109:3: note: expanded from macro 'dev_printk_index_wrap'
     109 |                 dev_printk_index_emit(level, fmt);                      \
         |                 ^
   include/linux/dev_printk.h:105:2: note: expanded from macro 'dev_printk_index_emit'
     105 |         printk_index_subsys_emit("%s %s: ", level, fmt)
         |         ^
   include/linux/printk.h:479:2: note: expanded from macro 'printk_index_subsys_emit'
     479 |         __printk_index_emit(fmt, level, subsys_fmt_prefix)
         |         ^
   include/linux/printk.h:445:32: note: expanded from macro '__printk_index_emit'
     445 |                                 .fmt = __builtin_constant_p(_fmt) ? (_fmt) : NULL, \
         |                                                            ^
>> drivers/media/usb/ttusb-dec/ttusb_dec.c:713:7: error: expected ')'
     713 |                                                 __func__);
         |                                                 ^
   drivers/media/usb/ttusb-dec/ttusb_dec.c:711:6: note: to match this '('
     711 |                                         dev_warn(&dec->udev->dev,
         |                                         ^
   include/linux/dev_printk.h:156:2: note: expanded from macro 'dev_warn'
     156 |         dev_printk_index_wrap(_dev_warn, KERN_WARNING, dev, dev_fmt(fmt), ##__VA_ARGS__)
         |         ^
   include/linux/dev_printk.h:109:3: note: expanded from macro 'dev_printk_index_wrap'
     109 |                 dev_printk_index_emit(level, fmt);                      \
         |                 ^
   include/linux/dev_printk.h:105:2: note: expanded from macro 'dev_printk_index_emit'
     105 |         printk_index_subsys_emit("%s %s: ", level, fmt)
         |         ^
   include/linux/printk.h:479:2: note: expanded from macro 'printk_index_subsys_emit'
     479 |         __printk_index_emit(fmt, level, subsys_fmt_prefix)
         |         ^
   include/linux/printk.h:445:41: note: expanded from macro '__printk_index_emit'
     445 |                                 .fmt = __builtin_constant_p(_fmt) ? (_fmt) : NULL, \
         |                                                                     ^
>> drivers/media/usb/ttusb-dec/ttusb_dec.c:713:7: error: expected ')'
     713 |                                                 __func__);
         |                                                 ^
   drivers/media/usb/ttusb-dec/ttusb_dec.c:711:6: note: to match this '('
     711 |                                         dev_warn(&dec->udev->dev,
         |                                         ^
   include/linux/dev_printk.h:156:2: note: expanded from macro 'dev_warn'
     156 |         dev_printk_index_wrap(_dev_warn, KERN_WARNING, dev, dev_fmt(fmt), ##__VA_ARGS__)
         |         ^
   include/linux/dev_printk.h:110:10: note: expanded from macro 'dev_printk_index_wrap'
     110 |                 _p_func(dev, fmt, ##__VA_ARGS__);                       \
         |                        ^
   4 errors generated.


vim +713 drivers/media/usb/ttusb-dec/ttusb_dec.c

   640	
   641	static void ttusb_dec_process_urb_frame(struct ttusb_dec *dec, u8 *b,
   642						int length)
   643	{
   644		swap_bytes(b, length);
   645	
   646		while (length) {
   647			switch (dec->packet_state) {
   648	
   649			case 0:
   650			case 1:
   651			case 2:
   652				if (*b++ == 0xaa)
   653					dec->packet_state++;
   654				else
   655					dec->packet_state = 0;
   656	
   657				length--;
   658				break;
   659	
   660			case 3:
   661				if (*b == 0x00) {
   662					dec->packet_state++;
   663					dec->packet_length = 0;
   664				} else if (*b != 0xaa) {
   665					dec->packet_state = 0;
   666				}
   667	
   668				b++;
   669				length--;
   670				break;
   671	
   672			case 4:
   673				dec->packet[dec->packet_length++] = *b++;
   674	
   675				if (dec->packet_length == 2) {
   676					if (dec->packet[0] == 'A' &&
   677					    dec->packet[1] == 'V') {
   678						dec->packet_type =
   679							TTUSB_DEC_PACKET_PVA;
   680						dec->packet_state++;
   681					} else if (dec->packet[0] == 'S') {
   682						dec->packet_type =
   683							TTUSB_DEC_PACKET_SECTION;
   684						dec->packet_state++;
   685					} else if (dec->packet[0] == 0x00) {
   686						dec->packet_type =
   687							TTUSB_DEC_PACKET_EMPTY;
   688						dec->packet_payload_length = 2;
   689						dec->packet_state = 7;
   690					} else {
   691						printk("%s: unknown packet type: %02x%02x\n",
   692						       __func__,
   693						       dec->packet[0], dec->packet[1]);
   694						dec->packet_state = 0;
   695					}
   696				}
   697	
   698				length--;
   699				break;
   700	
   701			case 5:
   702				dec->packet[dec->packet_length++] = *b++;
   703	
   704				if (dec->packet_type == TTUSB_DEC_PACKET_PVA &&
   705				    dec->packet_length == 8) {
   706					int len = 8 +
   707						(dec->packet[6] << 8) +
   708						dec->packet[7];
   709	
   710					if (len > MAX_PVA_LENGTH + 4) {
   711						dev_warn(&dec->udev->dev,
   712							"%s: packet too long - discarding\n"
 > 713							__func__);
   714						dec->packet_state = 0;
   715					} else {
   716						dec->packet_state++;
   717						dec->packet_payload_length = len;
   718					}
   719				} else if (dec->packet_type ==
   720						TTUSB_DEC_PACKET_SECTION &&
   721					   dec->packet_length == 5) {
   722					dec->packet_state++;
   723					dec->packet_payload_length = 5 +
   724						((dec->packet[3] & 0x0f) << 8) +
   725						dec->packet[4];
   726				}
   727	
   728				length--;
   729				break;
   730	
   731			case 6: {
   732				int remainder = dec->packet_payload_length -
   733						dec->packet_length;
   734	
   735				if (length >= remainder) {
   736					memcpy(dec->packet + dec->packet_length,
   737					       b, remainder);
   738					dec->packet_length += remainder;
   739					b += remainder;
   740					length -= remainder;
   741					dec->packet_state++;
   742				} else {
   743					memcpy(&dec->packet[dec->packet_length],
   744					       b, length);
   745					dec->packet_length += length;
   746					length = 0;
   747				}
   748	
   749				break;
   750			}
   751	
   752			case 7: {
   753				int tail = 4;
   754	
   755				dec->packet[dec->packet_length++] = *b++;
   756	
   757				if (dec->packet_type == TTUSB_DEC_PACKET_SECTION &&
   758				    dec->packet_payload_length % 2)
   759					tail++;
   760	
   761				if (dec->packet_length ==
   762				    dec->packet_payload_length + tail) {
   763					ttusb_dec_process_packet(dec);
   764					dec->packet_state = 0;
   765				}
   766	
   767				length--;
   768				break;
   769			}
   770	
   771			default:
   772				printk("%s: illegal packet state encountered.\n",
   773				       __func__);
   774				dec->packet_state = 0;
   775			}
   776		}
   777	}
   778	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

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

* [PATCH v4] media: ttusb-dec: fix heap-buffer-overflow in ttusb_dec_process_urb_frame()
  2025-12-23  1:01   ` [PATCH v3] " pip-izony
@ 2025-12-30 19:50     ` pip-izony
  0 siblings, 0 replies; 6+ messages in thread
From: pip-izony @ 2025-12-30 19:50 UTC (permalink / raw)
  To: Mauro Carvalho Chehab
  Cc: Seungjin Bae, Kyungtae Kim, Sanghoon Choi, linux-media,
	linux-kernel

From: Seungjin Bae <eeodqql09@gmail.com>

The `ttusb_dec_process_urb_frame()` parses the PVA packet from the
USB device. However, it doesn't check whether the calculated
`packet_payload_length` exceeds the size of the `packet` buffer.

The `packet` buffer has a fixed size of `MAX_PVA_LENGTH + 4`. However,
`packet_payload_length` is derived from 2 bytes of the input data,
allowing a maximum value of 65543 bytes (8 + 0xFFFF).

If a malicious USB device sends a packet with crafted data, it
triggers a heap buffer overflow. This allows an attacker to overwrite
adjacent fields in the `struct ttusb_dec`. Specifically, the `a_pes2ts`
field, which contains a callback function pointer, is located after the
`packet` buffer. Overwriting this pointer can lead to control flow
hijacking.

Fix this by adding a bounds check for the parsed length against the
buffer size.

Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Co-developed-by: Sanghoon Choi <csh0052@gmail.com>
Signed-off-by: Sanghoon Choi <csh0052@gmail.com>
Signed-off-by: Seungjin Bae <eeodqql09@gmail.com>
---
 v1 -> v2: Change warning function
 v2 -> v3: Add missing comma in the dev_warn argument
 v3 -> v4: Edit alignment
 
 drivers/media/usb/ttusb-dec/ttusb_dec.c | 13 +++++++++++--
 1 file changed, 11 insertions(+), 2 deletions(-)

diff --git a/drivers/media/usb/ttusb-dec/ttusb_dec.c b/drivers/media/usb/ttusb-dec/ttusb_dec.c
index b4575fe89c95..17c7a8d5ada9 100644
--- a/drivers/media/usb/ttusb-dec/ttusb_dec.c
+++ b/drivers/media/usb/ttusb-dec/ttusb_dec.c
@@ -703,10 +703,19 @@ static void ttusb_dec_process_urb_frame(struct ttusb_dec *dec, u8 *b,
 
 			if (dec->packet_type == TTUSB_DEC_PACKET_PVA &&
 			    dec->packet_length == 8) {
-				dec->packet_state++;
-				dec->packet_payload_length = 8 +
+				int len = 8 +
 					(dec->packet[6] << 8) +
 					dec->packet[7];
+
+				if (len > MAX_PVA_LENGTH + 4) {
+					dev_warn(&dec->udev->dev,
+						 "%s: packet too long - discarding\n",
+						 __func__);
+					dec->packet_state = 0;
+				} else {
+					dec->packet_state++;
+					dec->packet_payload_length = len;
+				}
 			} else if (dec->packet_type ==
 					TTUSB_DEC_PACKET_SECTION &&
 				   dec->packet_length == 5) {
-- 
2.43.0


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

end of thread, other threads:[~2025-12-30 19:51 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-12-22  0:20 [PATCH] media: ttusb-dec: fix heap-buffer-overflow in ttusb_dec_process_urb_frame() pip-izony
2025-12-22  5:46 ` [PATCH v2] " pip-izony
2025-12-23  0:31   ` kernel test robot
2025-12-23  1:01   ` [PATCH v3] " pip-izony
2025-12-30 19:50     ` [PATCH v4] " pip-izony
2025-12-23  1:17   ` [PATCH v2] " kernel test robot

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).