[vlc-commits] [Git][videolan/vlc][master] 3 commits: demux: mkv: do not use more buttons than defined in pci.hli.btnit

Steve Lhomme (@robUx4) gitlab at videolan.org
Sat Jun 20 08:32:33 UTC 2026



Steve Lhomme pushed to branch master at VideoLAN / VLC


Commits:
3427af03 by Steve Lhomme at 2026-06-20T08:20:11+00:00
demux: mkv: do not use more buttons than defined in pci.hli.btnit

Fixes #29772

Reported-By: tianshuo han <hantianshuo233 at gmail.com>

- - - - -
09167358 by Steve Lhomme at 2026-06-20T08:20:11+00:00
demux: mkv: read the DVD PCI packet using the bitstream (BS) reader

We should not be reading packed data straight into memory anyway.
We can also reduce the structure size by removing some unused bytes.

- - - - -
1b6a9873 by Steve Lhomme at 2026-06-20T08:20:11+00:00
demux: mkv: add more sanity checks on button index values

- - - - -


2 changed files:

- modules/demux/mkv/chapter_command_dvd.cpp
- modules/demux/mkv/dvd_types.hpp


Changes:

=====================================
modules/demux/mkv/chapter_command_dvd.cpp
=====================================
@@ -10,6 +10,7 @@
 #include "virtual_segment.hpp"
 
 #include <vlc_subpicture.h> // vlc_spu_highlight_t
+#include <vlc_bits.h>
 
 namespace mkv {
 
@@ -574,7 +575,7 @@ bool dvd_command_interpretor_c::ProcessNavAction( uint16_t button )
 {
     const pci_t & pci = pci_packet;
 
-    if( button <= 0 || button > pci.hli.hl_gi.btn_ns )
+    if( button == 0 || button > pci.hli.hl_gi.btn_ns || button > ARRAY_SIZE(pci.hli.btnit) )
         return false;
 
     SetSPRM( 0x88, button );
@@ -592,7 +593,7 @@ bool dvd_command_interpretor_c::HandleKeyEvent( NavivationKey key )
     const pci_t & pci = pci_packet;
     uint16_t i_curr_button = GetSPRM( 0x88 );
 
-    if( i_curr_button <= 0 || i_curr_button > pci.hli.hl_gi.btn_ns )
+    if( i_curr_button == 0 || i_curr_button > pci.hli.hl_gi.btn_ns || i_curr_button > ARRAY_SIZE(pci.hli.btnit) )
         return false;
 
     const btni_t & button_ptr = pci.hli.btnit[i_curr_button-1];
@@ -617,14 +618,14 @@ void dvd_command_interpretor_c::HandleMousePressed( unsigned x, unsigned y )
 {
     const pci_t & pci = pci_packet;
 
-    int32_t button;
-    int32_t best,dist,d;
+    uint16_t best;
+    uint32_t dist,d;
     int32_t mx,my,dx,dy;
 
     // get current button
     best = 0;
-    dist = 0x08000000; /* >> than  (720*720)+(567*567); */
-    for(button = 1; button <= pci.hli.hl_gi.btn_ns; button++)
+    dist = std::numeric_limits<uint32_t>::max(); /* >> than  (720*720)+(567*567); */
+    for(uint16_t button = 1; button <= std::min<uint16_t>(pci.hli.hl_gi.btn_ns, ARRAY_SIZE(pci.hli.btnit)); button++)
     {
         const btni_t & button_ptr = pci.hli.btnit[button-1];
 
@@ -653,13 +654,13 @@ void dvd_command_interpretor_c::HandleMousePressed( unsigned x, unsigned y )
     const btni_t & button_ptr = pci.hli.btnit[best-1];
     uint16_t i_curr_button = GetSPRM( 0x88 );
 
-    vlc_debug( l, "Clicked button %d", best );
+    vlc_debug( l, "Clicked button %" PRIu16, best );
 
     // process the button action
     SetSPRM( 0x88, best );
     Interpret( MATROSKA_CHAPPROCESSTIME_DURING, button_ptr.cmd.bytes, 8 );
 
-    vlc_debug( l, "Processed button %d", best );
+    vlc_debug( l, "Processed button %" PRIu16, best );
 
     // select new button
     if ( best != i_curr_button )
@@ -803,31 +804,114 @@ bool dvd_command_interpretor_c::MatchCellNumber( const chapter_codec_cmds_c &dat
 
 void dvd_command_interpretor_c::SetPci(const uint8_t *data, unsigned size)
 {
-    if (size < sizeof(pci_packet))
+    if (size < PCI_RAW_SIZE)
         return;
 
-    memcpy(&pci_packet, data, sizeof(pci_packet));
-
-#ifndef WORDS_BIGENDIAN
-    for( uint8_t button = 1; button <= pci_packet.hli.hl_gi.btn_ns &&
-            button < ARRAY_SIZE(pci_packet.hli.btnit); button++) {
-        btni_t & button_ptr = pci_packet.hli.btnit[button-1];
-        binary *p_data = (binary*) &button_ptr;
-
-        uint16_t i_x_start = ((p_data[0] & 0x3F) << 4 ) + ( p_data[1] >> 4 );
-        uint16_t i_x_end   = ((p_data[1] & 0x03) << 8 ) + p_data[2];
-        uint16_t i_y_start = ((p_data[3] & 0x3F) << 4 ) + ( p_data[4] >> 4 );
-        uint16_t i_y_end   = ((p_data[4] & 0x03) << 8 ) + p_data[5];
-        button_ptr.x_start = i_x_start;
-        button_ptr.x_end   = i_x_end;
-        button_ptr.y_start = i_y_start;
-        button_ptr.y_end   = i_y_end;
-
+    bs_t bs;
+    bs_init(&bs, data, size);
+
+    pci_packet.pci_gi.nv_pck_lbn = bs_read(&bs, 32);
+    pci_packet.pci_gi.vobu_cat   = bs_read(&bs, 16);
+    bs_skip(&bs, 16);
+
+    bs_skip(&bs, 7);
+    pci_packet.pci_gi.vobu_uop_ctl.video_pres_mode_change = bs_read1(&bs);
+
+    pci_packet.pci_gi.vobu_uop_ctl.karaoke_audio_pres_mode_change = bs_read1(&bs);
+    pci_packet.pci_gi.vobu_uop_ctl.angle_change           = bs_read1(&bs);
+    pci_packet.pci_gi.vobu_uop_ctl.subpic_stream_change   = bs_read1(&bs);
+    pci_packet.pci_gi.vobu_uop_ctl.audio_stream_change    = bs_read1(&bs);
+    pci_packet.pci_gi.vobu_uop_ctl.pause_on               = bs_read1(&bs);
+    pci_packet.pci_gi.vobu_uop_ctl.still_off              = bs_read1(&bs);
+    pci_packet.pci_gi.vobu_uop_ctl.button_select_or_activate = bs_read1(&bs);
+    pci_packet.pci_gi.vobu_uop_ctl.resume                 = bs_read1(&bs);
+
+    pci_packet.pci_gi.vobu_uop_ctl.chapter_menu_call      = bs_read1(&bs);
+    pci_packet.pci_gi.vobu_uop_ctl.angle_menu_call        = bs_read1(&bs);
+    pci_packet.pci_gi.vobu_uop_ctl.audio_menu_call        = bs_read1(&bs);
+    pci_packet.pci_gi.vobu_uop_ctl.subpic_menu_call       = bs_read1(&bs);
+    pci_packet.pci_gi.vobu_uop_ctl.root_menu_call         = bs_read1(&bs);
+    pci_packet.pci_gi.vobu_uop_ctl.title_menu_call        = bs_read1(&bs);
+    pci_packet.pci_gi.vobu_uop_ctl.backward_scan          = bs_read1(&bs);
+    pci_packet.pci_gi.vobu_uop_ctl.forward_scan           = bs_read1(&bs);
+
+    pci_packet.pci_gi.vobu_uop_ctl.next_pg_search         = bs_read1(&bs);
+    pci_packet.pci_gi.vobu_uop_ctl.prev_or_top_pg_search  = bs_read1(&bs);
+    pci_packet.pci_gi.vobu_uop_ctl.time_or_chapter_search = bs_read1(&bs);
+    pci_packet.pci_gi.vobu_uop_ctl.go_up                  = bs_read1(&bs);
+    pci_packet.pci_gi.vobu_uop_ctl.stop                   = bs_read1(&bs);
+    pci_packet.pci_gi.vobu_uop_ctl.title_play             = bs_read1(&bs);
+    pci_packet.pci_gi.vobu_uop_ctl.chapter_search_or_play = bs_read1(&bs);
+    pci_packet.pci_gi.vobu_uop_ctl.title_or_time_play     = bs_read1(&bs);
+
+    pci_packet.pci_gi.vobu_s_ptm    = bs_read(&bs, 32);
+    pci_packet.pci_gi.vobu_e_ptm    = bs_read(&bs, 32);
+    pci_packet.pci_gi.vobu_se_e_ptm = bs_read(&bs, 32);
+
+    pci_packet.pci_gi.e_eltm.hour    = bs_read(&bs, 8);
+    pci_packet.pci_gi.e_eltm.minute  = bs_read(&bs, 8);
+    pci_packet.pci_gi.e_eltm.second  = bs_read(&bs, 8);
+    pci_packet.pci_gi.e_eltm.frame_u = bs_read(&bs, 8);
+
+    // FIXME read buffer in one pass using bs_t
+    for (size_t i = 0; i < ARRAY_SIZE(pci_packet.pci_gi.vobu_isrc); i++)
+        pci_packet.pci_gi.vobu_isrc[i] = bs_read(&bs, 8);
+
+    for (size_t i = 0; i < ARRAY_SIZE(pci_packet.nsml_agli.nsml_agl_dsta); i++)
+        pci_packet.nsml_agli.nsml_agl_dsta[i] = bs_read(&bs, 32);
+
+    pci_packet.hli.hl_gi.hli_ss       = bs_read(&bs, 16);
+    pci_packet.hli.hl_gi.hli_s_ptm    = bs_read(&bs, 32);
+    pci_packet.hli.hl_gi.hli_e_ptm    = bs_read(&bs, 32);
+    pci_packet.hli.hl_gi.btn_se_e_ptm = bs_read(&bs, 32);
+    bs_skip(&bs, 2);
+    pci_packet.hli.hl_gi.btngr_ns      = bs_read(&bs, 2);
+    bs_skip(&bs, 1);
+    pci_packet.hli.hl_gi.btngr1_dsp_ty = bs_read(&bs, 3);
+    bs_skip(&bs, 1);
+    pci_packet.hli.hl_gi.btngr2_dsp_ty = bs_read(&bs, 3);
+    bs_skip(&bs, 1);
+    pci_packet.hli.hl_gi.btngr3_dsp_ty = bs_read(&bs, 3);
+    pci_packet.hli.hl_gi.btn_ofn       = bs_read(&bs, 8);
+    pci_packet.hli.hl_gi.btn_ns        = bs_read(&bs, 8);
+    pci_packet.hli.hl_gi.nsl_btn_ns    = bs_read(&bs, 8);
+    bs_skip(&bs, 8);
+    pci_packet.hli.hl_gi.fosl_btnn     = bs_read(&bs, 8);
+    pci_packet.hli.hl_gi.foac_btnn     = bs_read(&bs, 8);
+
+    for (size_t i = 0; i < 3; i++)
+        for (size_t j = 0; j < 2; j++)
+            pci_packet.hli.btn_colit.btn_coli[i][j] = bs_read(&bs, 32 );
+
+    for (size_t i = 0; i < ARRAY_SIZE(pci_packet.hli.btnit); i++)
+    {
+        auto & btn = pci_packet.hli.btnit[i];
+        btn.btn_coln = bs_read(&bs, 2);
+        btn.x_start  = bs_read(&bs, 10);
+        bs_skip(&bs, 2);
+        btn.x_end    = bs_read(&bs, 10);
+
+        bs_skip(&bs, 2);
+        btn.up = bs_read(&bs, 6);
+
+        btn.auto_action_mode = bs_read(&bs, 2);
+        btn.y_start = bs_read(&bs, 10);
+        bs_skip(&bs, 2);
+        btn.y_end   = bs_read(&bs, 10);
+
+        bs_skip(&bs, 2);
+        btn.down    = bs_read(&bs, 6);
+        bs_skip(&bs, 2);
+        btn.left    = bs_read(&bs, 6);
+        bs_skip(&bs, 2);
+        btn.right   = bs_read(&bs, 6);
+
+        // FIXME read buffer in one pass using bs_t
+        for (size_t i = 0; i < ARRAY_SIZE(btn.cmd.bytes); i++)
+            btn.cmd.bytes[i] = bs_read(&bs, 8);
     }
-    for ( uint8_t i = 0; i<3; i++ )
-        for ( uint8_t j = 0; j<2; j++ )
-            pci_packet.hli.btn_colit.btn_coli[i][j] = U32_AT( &pci_packet.hli.btn_colit.btn_coli[i][j] );
-#endif
+    bs_skip(&bs, 8*189);
+    assert(!bs_error(&bs));
 }
 
 } // namespace


=====================================
modules/demux/mkv/dvd_types.hpp
=====================================
@@ -28,23 +28,6 @@
 
 namespace mkv {
 ////////////////////////////////////////////////////////////////////////////////////////////////////////////////
-#undef ATTRIBUTE_PACKED
-#undef PRAGMA_PACK_BEGIN
-#undef PRAGMA_PACK_END
-
-#if defined(__GNUC__)
-#define ATTRIBUTE_PACKED __attribute__ ((packed))
-#define PRAGMA_PACK 0
-#endif
-
-#if !defined(ATTRIBUTE_PACKED)
-#define ATTRIBUTE_PACKED
-#define PRAGMA_PACK 1
-#endif
-
-#if PRAGMA_PACK
-#pragma pack(1)
-#endif
 
 /*************************************
 *  taken from libdvdnav / libdvdread
@@ -53,21 +36,20 @@ namespace mkv {
 /**
  * DVD Time Information.
  */
-typedef struct {
+struct dvd_time_t {
   uint8_t hour;
   uint8_t minute;
   uint8_t second;
   uint8_t frame_u; /* The two high bits are the frame rate. */
-} ATTRIBUTE_PACKED dvd_time_t;
+};
 
 /**
  * User Operations.
  */
-typedef struct {
-#ifdef WORDS_BIGENDIAN
+struct user_ops_t {
   unsigned char zero                           : 7; /* 25-31 */
   unsigned char video_pres_mode_change         : 1; /* 24 */
- 
+
   unsigned char karaoke_audio_pres_mode_change : 1; /* 23 */
   unsigned char angle_change                   : 1;
   unsigned char subpic_stream_change           : 1;
@@ -76,7 +58,7 @@ typedef struct {
   unsigned char still_off                      : 1;
   unsigned char button_select_or_activate      : 1;
   unsigned char resume                         : 1; /* 16 */
- 
+
   unsigned char chapter_menu_call              : 1; /* 15 */
   unsigned char angle_menu_call                : 1;
   unsigned char audio_menu_call                : 1;
@@ -85,7 +67,7 @@ typedef struct {
   unsigned char title_menu_call                : 1;
   unsigned char backward_scan                  : 1;
   unsigned char forward_scan                   : 1; /* 8 */
- 
+
   unsigned char next_pg_search                 : 1; /* 7 */
   unsigned char prev_or_top_pg_search          : 1;
   unsigned char time_or_chapter_search         : 1;
@@ -94,68 +76,37 @@ typedef struct {
   unsigned char title_play                     : 1;
   unsigned char chapter_search_or_play         : 1;
   unsigned char title_or_time_play             : 1; /* 0 */
-#else
-  unsigned char video_pres_mode_change         : 1; /* 24 */
-  unsigned char zero                           : 7; /* 25-31 */
- 
-  unsigned char resume                         : 1; /* 16 */
-  unsigned char button_select_or_activate      : 1;
-  unsigned char still_off                      : 1;
-  unsigned char pause_on                       : 1;
-  unsigned char audio_stream_change            : 1;
-  unsigned char subpic_stream_change           : 1;
-  unsigned char angle_change                   : 1;
-  unsigned char karaoke_audio_pres_mode_change : 1; /* 23 */
- 
-  unsigned char forward_scan                   : 1; /* 8 */
-  unsigned char backward_scan                  : 1;
-  unsigned char title_menu_call                : 1;
-  unsigned char root_menu_call                 : 1;
-  unsigned char subpic_menu_call               : 1;
-  unsigned char audio_menu_call                : 1;
-  unsigned char angle_menu_call                : 1;
-  unsigned char chapter_menu_call              : 1; /* 15 */
- 
-  unsigned char title_or_time_play             : 1; /* 0 */
-  unsigned char chapter_search_or_play         : 1;
-  unsigned char title_play                     : 1;
-  unsigned char stop                           : 1;
-  unsigned char go_up                          : 1;
-  unsigned char time_or_chapter_search         : 1;
-  unsigned char prev_or_top_pg_search          : 1;
-  unsigned char next_pg_search                 : 1; /* 7 */
-#endif
-} ATTRIBUTE_PACKED user_ops_t;
+};
 
 /**
  * Type to store per-command data.
  */
-typedef struct {
+struct vm_cmd_t {
   uint8_t bytes[8];
-} ATTRIBUTE_PACKED vm_cmd_t;
+};
 #define COMMAND_DATA_SIZE 8
 
 /**
  * PCI General Information
  */
-typedef struct {
+struct pci_gi_t {
   uint32_t nv_pck_lbn;      /**< sector address of this nav pack */
   uint16_t vobu_cat;        /**< 'category' of vobu */
-  uint16_t zero1;           /**< reserved */
+  // uint16_t zero1;           /**< reserved */
   user_ops_t vobu_uop_ctl;  /**< UOP of vobu */
   uint32_t vobu_s_ptm;      /**< start presentation time of vobu */
   uint32_t vobu_e_ptm;      /**< end presentation time of vobu */
   uint32_t vobu_se_e_ptm;   /**< end ptm of sequence end in vobu */
   dvd_time_t e_eltm;        /**< Cell elapsed time */
   char vobu_isrc[32];
-} ATTRIBUTE_PACKED pci_gi_t;
+};
 
 /**
  * Non Seamless Angle Information
  */
-typedef struct {
+struct nsml_agli_t {
   uint32_t nsml_agl_dsta[9];  /**< address of destination vobu in AGL_C#n */
-} ATTRIBUTE_PACKED nsml_agli_t;
+};
 
 /**
  * Highlight General Information
@@ -166,12 +117,11 @@ typedef struct {
  * X1Xb: letterbox buttons
  * 1XXb: pan&scan buttons
  */
-typedef struct {
+struct hl_gi_t {
   uint16_t hli_ss; /**< status, only low 2 bits 0: no buttons, 1: different 2: equal 3: eual except for button cmds */
   uint32_t hli_s_ptm;              /**< start ptm of hli */
   uint32_t hli_e_ptm;              /**< end ptm of hli */
   uint32_t btn_se_e_ptm;           /**< end ptm of button select */
-#ifdef WORDS_BIGENDIAN
   unsigned char zero1 : 2;          /**< reserved */
   unsigned char btngr_ns : 2;       /**< number of button groups 1, 2 or 3 with 36/18/12 buttons */
   unsigned char zero2 : 1;          /**< reserved */
@@ -180,24 +130,13 @@ typedef struct {
   unsigned char btngr2_dsp_ty : 3;  /**< display type of subpic stream for button group 2 */
   unsigned char zero4 : 1;          /**< reserved */
   unsigned char btngr3_dsp_ty : 3;  /**< display type of subpic stream for button group 3 */
-#else
-  unsigned char btngr1_dsp_ty : 3;
-  unsigned char zero2 : 1;
-  unsigned char btngr_ns : 2;
-  unsigned char zero1 : 2;
-  unsigned char btngr3_dsp_ty : 3;
-  unsigned char zero4 : 1;
-  unsigned char btngr2_dsp_ty : 3;
-  unsigned char zero3 : 1;
-#endif
   uint8_t btn_ofn;     /**< button offset number range 0-255 */
   uint8_t btn_ns;      /**< number of valid buttons  <= 36/18/12 (low 6 bits) */
   uint8_t nsl_btn_ns;  /**< number of buttons selectable by U_BTNNi (low 6 bits)   nsl_btn_ns <= btn_ns */
-  uint8_t zero5;       /**< reserved */
+  // uint8_t zero5;       /**< reserved */
   uint8_t fosl_btnn;   /**< forcedly selected button  (low 6 bits) */
   uint8_t foac_btnn;   /**< forcedly activated button (low 6 bits) */
-} ATTRIBUTE_PACKED hl_gi_t;
-
+};
 
 /**
  * Button Color Information Table
@@ -207,9 +146,9 @@ typedef struct {
  * that the indexes reference is in the PGC.
  * \todo split the uint32_t into a struct
  */
-typedef struct {
+struct btn_colit_t {
   uint32_t btn_coli[3][2];  /**< [button color number-1][select:0/action:1] */
-} ATTRIBUTE_PACKED btn_colit_t;
+};
 
 /**
  * Button Information
@@ -218,8 +157,7 @@ typedef struct {
  * the packing to work with Sun's Forte C compiler.
  * The 4 and 7 bytes are 'rotated' was: ABC DEF GHIJ  is: ABCG DEFH IJ
  */
-typedef struct {
-#ifdef WORDS_BIGENDIAN
+struct btni_t {
   uint32_t      btn_coln         : 2;  /**< button color number */
   uint32_t      x_start          : 10; /**< x start offset within the overlay */
   uint32_t      zero1            : 2;  /**< reserved */
@@ -239,52 +177,29 @@ typedef struct {
   unsigned char left             : 6;  /**< button index when pressing left */
   unsigned char zero6            : 2;  /**< reserved */
   unsigned char right            : 6;  /**< button index when pressing right */
-#else
-  uint32_t      x_end            : 10;
-  uint32_t      zero1            : 2;
-  uint32_t      x_start          : 10;
-  uint32_t      btn_coln         : 2;
-
-  uint32_t      up               : 6;
-  uint32_t      zero3            : 2;
-
-  uint32_t      y_end            : 10;
-  uint32_t      zero2            : 2;
-  uint32_t      y_start          : 10;
-  uint32_t      auto_action_mode : 2;
-
-  uint32_t      down             : 6;
-  uint32_t      zero4            : 2;
-  unsigned char left             : 6;
-  unsigned char zero5            : 2;
-  unsigned char right            : 6;
-  unsigned char zero6            : 2;
-#endif
   vm_cmd_t cmd;
-} ATTRIBUTE_PACKED btni_t;
+};
 
 /**
  * Highlight Information
  */
-typedef struct {
+struct hli_t {
   hl_gi_t     hl_gi;
   btn_colit_t btn_colit;
   btni_t      btnit[36];
-} ATTRIBUTE_PACKED hli_t;
+};
 
 /**
  * PCI packet
  */
-typedef struct pci_t {
+struct pci_t {
   pci_gi_t    pci_gi;
   nsml_agli_t nsml_agli;
   hli_t       hli;
-  uint8_t     zero1[189];
-} ATTRIBUTE_PACKED pci_t;
+  // uint8_t     zero1[189];
+};
+#define PCI_RAW_SIZE 1051U
 
-#if PRAGMA_PACK
-#pragma pack()
-#endif
 
 } // namespace
 



View it on GitLab: https://code.videolan.org/videolan/vlc/-/compare/5cb22e56bc018c2a7f3bd84b3a566f90ae16593c...1b6a987363f8c2ac0c99d7e7067ae827e5ebfa76

-- 
View it on GitLab: https://code.videolan.org/videolan/vlc/-/compare/5cb22e56bc018c2a7f3bd84b3a566f90ae16593c...1b6a987363f8c2ac0c99d7e7067ae827e5ebfa76
You're receiving this email because of your account on code.videolan.org. Manage all notifications: https://code.videolan.org/-/profile/notifications | Help: https://code.videolan.org/help




More information about the vlc-commits mailing list