[PATCH] Race conditions on shutdown

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

 



Wed, Nov 30, 2022 at 02:01:13PM +0100, Klaus Schmidinger wrote:
VDR version 2.6.2 is now available at the official VDR GIT archive

     git://git.tvdr.de

Thank you, Klaus!

While debugging hangs or crashes during the shutdown of rpihddevice (see https://github.com/reufer/rpihddevice/pull/6 for my fixes), I noticed that AddressSanitizer (enabled by -fsanitize=address in GCC or clang) would report some heap-use-after-free also in
cDvbTuner::Action(), accessing a deleted cSdtFilter.

The attached patch fixes the issues for me. The Cancel(3) call in cDevice::~cDevice() is moved to cDevice::Shutdown(), because cDevice::~cDevice() would be executed too late, while cDvbDevice::~cDvbDevice() had already deleted the objects. The Cancel(-1) calls in cDevice::Shutdown() are an optimization to reduce the shutdown time on systems when there are multiple devices.

The change to cDvbDevice::~cDvbDevice() is necessary to shut down all users of cDvbTuner before it is deleted. Without this last hunk, I would still occasionally get heap-use-after-free reports.

	Marko
diff --git a/device.c b/device.c
index 4b9c9cc7..4e987389 100644
--- a/device.c
+++ b/device.c
@@ -125,7 +125,6 @@ cDevice::~cDevice()
   delete dvbSubtitleConverter;
   if (this == primaryDevice)
      primaryDevice = NULL;
-  Cancel(3);
 }
 
 bool cDevice::WaitForAllDevicesReady(int Timeout)
@@ -457,7 +456,10 @@ void cDevice::SetCamSlot(cCamSlot *CamSlot)
 void cDevice::Shutdown(void)
 {
   deviceHooks.Clear();
+  for (int i = 0; i < numDevices; i++)
+      device[i]->Cancel(-1);
   for (int i = 0; i < numDevices; i++) {
+      device[i]->Cancel(3);
       delete device[i];
       device[i] = NULL;
       }
diff --git a/dvbdevice.c b/dvbdevice.c
index 51331485..1caf1218 100644
--- a/dvbdevice.c
+++ b/dvbdevice.c
@@ -573,6 +573,7 @@ private:
   virtual void Action(void);
 public:
   cDvbTuner(const cDvbDevice *Device, int Adapter, int Frontend);
+  void Shutdown();
   virtual ~cDvbTuner();
   bool ProvidesDeliverySystem(int DeliverySystem) const;
   bool ProvidesModulation(int System, int StreamId, int Modulation) const;
@@ -648,12 +649,17 @@ cDvbTuner::cDvbTuner(const cDvbDevice *Device, int Adapter, int Frontend)
   Start();
 }
 
-cDvbTuner::~cDvbTuner()
+void cDvbTuner::Shutdown()
 {
   tunerStatus = tsIdle;
   newSet.Broadcast();
   locked.Broadcast();
   Cancel(3);
+}
+
+cDvbTuner::~cDvbTuner()
+{
+  Shutdown();
   UnBond();
   /* looks like this irritates the SCR switch, so let's leave it out for now
   if (lastDiseqc && lastDiseqc->IsScr()) {
@@ -1865,6 +1871,8 @@ cDvbDevice::cDvbDevice(int Adapter, int Frontend)
 
 cDvbDevice::~cDvbDevice()
 {
+  if (dvbTuner)
+     dvbTuner->Shutdown();
   StopSectionHandler();
   delete dvbTuner;
   delete ciAdapter;
_______________________________________________
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