Re: [PATCH] Fix undefined behaviour

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

 



Mon, Dec 05, 2022 at 04:08:45PM +0100, Klaus Schmidinger wrote:
Instead if typecasting I guess I'll rather do it this way:

This worked as well.

If x2 ever becomes negative, something else must have gone wrong.

The actual culprit is cDvbSubtitleConverter::FinishPage(), which was invoking this code with NumAreas == 0 and a null pointer Areas. After I fixed that, the error went away, and Finnish DVB subtitles were still being displayed.

The first attached patch includes your suggested fixes and nothing that you opposed so far. The second attached patch fixes the following 2 issues. I agree that the NumCamSlots==0 case could be solved in a nicer way.

If (rows * pitch) is 0, nothing is copied.

On my test run, this code was not hit until the end, upon pressing the Setup button (which was followed by Right, OK, OK, to shut down VDR):

#7 0x00278908 in cGlyph::cGlyph (this=0xc6a798, CharCode=32, GlyphData=<optimized out>) at font.c:77 #8 0x0027b660 in cFreetypeFont::Glyph (this=this@entry=0xc91ea8, CharCode=3220926570, CharCode@entry=3322475946, AntiAliased=<optimized out>) at font.c:231 #9 0x0027c21c in cFreetypeFont::Width (s=<optimized out>, this=<optimized out>) at font.c:261
#10 cFreetypeFont::Width (this=0xc91ea8, s=<optimized out>) at font.c:248
#11 0x00584754 in cSkinLCARSDisplayMenu::SetTitle (this=0xc7a1e8, Title=0x5d9 <error: Cannot access memory at address 0x5d9>)
    at skinlcars.c:1559
#12 0x00407f4c in cOsdMenu::Display (this=0xc95178) at osdbase.c:244
#13 0x004137dc in cOsdMenu::AddSubMenu (this=this@entry=0xc13ef8, SubMenu=SubMenu@entry=0xc95178) at osdbase.c:521 #14 0x00357ca0 in cMenuMain::cMenuMain (this=0xc95db8, State=<optimized out>, OpenSubMenus=<optimized out>) at menu.c:4489
#15 0x0007c9f0 in main (argc=<optimized out>, argv=<optimized out>)
    at vdr.c:1276

I guess that the bitmap for character code 32 (space) is empty. I dug a bit further in another run to find the string in question:

#12 0x00407f2c in cOsdMenu::Display (this=0xc592c8) at osdbase.c:244
244	  displayMenu->SetTitle(title);
(gdb) p *this
..., title = 0xc68400 "Asetukset - VDR 2.6.2", ...

That is the title of the Setup menu in Finnish. I cannot imagine any other fix of this.

The font was DejaVuSans-Bold.ttf, in case it makes a difference.

If NumCamSlots is 0, SlotPriority[] is never accessed.

True, but as I noted in my other reply, the compiler is actually allowed to assume that it will be accessed, for the first iteration of the second loop. I did not check the generated code for any normal optimized build to see whether my compilers would actually apply that optimization.

#6  0x7679c26c in __ubsan_handle_vla_bound_not_positive ()
   from /lib/arm-linux-gnueabihf/libubsan.so.1
#7 0x0016fb8c in cDevice::GetDevice (Channel=Channel@entry=0xbcb2a8, Priority=Priority@entry=0, LiveView=LiveView@entry=true, Query=Query@entry=false) at device.c:292 #8 0x0017e1e0 in cDevice::SetChannel (this=this@entry=0xbf5cf0, Channel=Channel@entry=0xbcb2a8, LiveView=LiveView@entry=125)
    at device.c:942
#9 0x0017fbb8 in cDevice::SwitchChannel (this=0xbf5cf0, Channel=0xbcb2a8, LiveView=LiveView@entry=true) at device.c:815
#10 0x000acfb8 in cChannels::SwitchTo (
    this=this@entry=0xaa9044 <cChannels::channels>, Number=<optimized out>)
    at device.h:148
#11 0x0007991c in main (argc=<optimized out>, argv=<optimized out>)
    at vdr.c:918

Best regards,

	Marko
diff --git a/dvbplayer.c b/dvbplayer.c
index 2ee846b6..b6620b5c 100644
--- a/dvbplayer.c
+++ b/dvbplayer.c
@@ -981,8 +981,10 @@ bool cDvbPlayer::GetReplayMode(bool &Play, bool &Forward, int &Speed)
 // --- cDvbPlayerControl -----------------------------------------------------
 
 cDvbPlayerControl::cDvbPlayerControl(const char *FileName, bool PauseLive)
-:cControl(player = new cDvbPlayer(FileName, PauseLive))
+:cControl(NULL, PauseLive)
 {
+  player = new cDvbPlayer(FileName, PauseLive);
+  SetPlayer(player);
 }
 
 cDvbPlayerControl::~cDvbPlayerControl()
diff --git a/dvbsubtitle.c b/dvbsubtitle.c
index c1dfef4d..2d22d963 100644
--- a/dvbsubtitle.c
+++ b/dvbsubtitle.c
@@ -1770,6 +1770,8 @@ void cDvbSubtitleConverter::FinishPage(cDvbSubtitlePage *Page)
      return;
   int NumAreas;
   tArea *Areas = Page->GetAreas(NumAreas);
+  if (!Areas)
+     return;
   tArea AreaCombined = Page->CombineAreas(NumAreas, Areas);
   tArea AreaOsd = Page->ScaleArea(AreaCombined, osdFactorX, osdFactorY);
   int Bpp = 8;
diff --git a/player.h b/player.h
index 22c748bd..d4b8f127 100644
--- a/player.h
+++ b/player.h
@@ -107,6 +107,7 @@ public:
          ///< Deletion of the marks themselves is handled separately, calling
          ///< this function merely tells the player to no longer display the
          ///< marks, if it has any.
+  void SetPlayer(cPlayer *Player) { player = Player; }
   double FramesPerSecond(void) const { return player ? player->FramesPerSecond() : DEFAULTFRAMESPERSECOND; }
   bool GetIndex(int &Current, int &Total, bool SnapToIFrame = false) const { return player ? player->GetIndex(Current, Total, SnapToIFrame) : false; }
   bool GetFrameNumber(int &Current, int &Total) const { return player ? player->GetFrameNumber(Current, Total) : false; }
diff --git a/sections.c b/sections.c
index 51a2823c..7a1ca8f5 100644
--- a/sections.c
+++ b/sections.c
@@ -180,6 +180,11 @@ void cSectionHandler::Action(void)
            startFilters = false;
            }
         int NumFilters = filterHandles.Count();
+        if (NumFilters == 0) {
+           Unlock();
+           cCondWait::SleepMs(100);
+           continue;
+           }
         pollfd pfd[NumFilters];
         for (cFilterHandle *fh = filterHandles.First(); fh; fh = filterHandles.Next(fh)) {
             int i = fh->Index();
diff --git a/transfer.c b/transfer.c
index 88931e58..dba1bf24 100644
--- a/transfer.c
+++ b/transfer.c
@@ -68,8 +68,10 @@ void cTransfer::Receive(const uchar *Data, int Length)
 cDevice *cTransferControl::receiverDevice = NULL;
 
 cTransferControl::cTransferControl(cDevice *ReceiverDevice, const cChannel *Channel)
-:cControl(transfer = new cTransfer(Channel), true)
+:cControl(NULL, true)
 {
+  transfer = new cTransfer(Channel);
+  SetPlayer(transfer);
   ReceiverDevice->AttachReceiver(transfer);
   receiverDevice = ReceiverDevice;
 }
diff --git a/device.c b/device.c
--- a/device.c
+++ b/device.c
@@ -249,7 +249,7 @@ cDevice *cDevice::GetDevice(const cChannel *Channel, int Priority, bool LiveView
 {
   // Collect the current priorities of all CAM slots that can decrypt the channel:
   int NumCamSlots = CamSlots.Count();
-  int SlotPriority[NumCamSlots];
+  int SlotPriority[std::max(1, NumCamSlots)];
   int NumUsableSlots = 0;
   bool InternalCamNeeded = false;
   if (Channel->Ca() >= CA_ENCRYPTED_MIN) {
diff --git a/font.c b/font.c
--- a/font.c
+++ b/font.c
@@ -74,7 +74,8 @@ cGlyph::cGlyph(uint CharCode, FT_GlyphSlotRec_ *GlyphData)
   rows = GlyphData->bitmap.rows;
   pitch = GlyphData->bitmap.pitch;
   bitmap = MALLOC(uchar, rows * pitch);
-  memcpy(bitmap, GlyphData->bitmap.buffer, rows * pitch);
+  if (int bytes = rows * pitch)
+     memcpy(bitmap, GlyphData->bitmap.buffer, bytes);
 }
 
 cGlyph::~cGlyph()
_______________________________________________
vdr mailing list
vdr@xxxxxxxxxxx
https://www.linuxtv.org/cgi-bin/mailman/listinfo/vdr

[Index of Archives]     [Linux Media]     [Asterisk]     [DCCP]     [Netdev]     [Xorg]     [Util Linux NG]     [Xfree86]     [Big List of Linux Books]     [Fedora Users]     [Fedora Women]     [ALSA Devel]     [Linux USB]

  Powered by Linux