From patchwork Sat Dec 3 14:20:16 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: =?utf-8?b?TWFya28gTcOka2Vsw6Q=?= X-Patchwork-Id: 87992 Received: from [127.0.0.1] by www.linuxtv.org with esmtp (Exim 4.92) (envelope-from ) id 1p1TNS-008iMX-Mk; Sat, 03 Dec 2022 14:20:26 +0000 Received: from meesny.iki.fi ([195.140.195.201]) by www.linuxtv.org with esmtps (TLS1.3:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.92) (envelope-from ) id 1p1TNN-008iM5-R7 for vdr@linuxtv.org; Sat, 03 Dec 2022 14:20:25 +0000 Received: from jyty (dsl-hkibng31-54fae6-199.dhcp.inet.fi [84.250.230.199]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange ECDHE (P-256) server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) (Authenticated sender: msmakela) by meesny.iki.fi (Postfix) with ESMTPSA id 6972420264 for ; Sat, 3 Dec 2022 16:20:17 +0200 (EET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=iki.fi; s=meesny; t=1670077217; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=7+zGd+8RvJAPXI+Kgrft4hjq47GPVC55X226AG3flcs=; b=N+GK7Z5YBNMCyRSydgZuu+0EaCdWatDtOnW0HpJJAgusbOoIE4oyMpkDH2t7ZpQ8jQk8Z4 4f++xqVAhEVWRKbp777ZCOUYn1Wd+c9DXDxqKmjvkoBcIxBVvmwAj/5gu+OpwV93DQ8XDz ayf25ce8UBjYLYuANUR247WgEJDBdmg= ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=iki.fi; s=meesny; t=1670077217; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=7+zGd+8RvJAPXI+Kgrft4hjq47GPVC55X226AG3flcs=; b=F20sXlhTzp5saiGJD+qlP9OZW7yrdeCfn4NAxJ2HO1dZqB2Eq8v6GpQ4A2kJiCFevkdLkc InULKaVzxOXTz4UhN74+iQFWCMcLO9AdIjXzxfZlkmvyETQd3gg9M8GV4MDAxPXHbZTHqB KomusWJasopbXwk28EV16rvjbE5octA= ARC-Seal: i=1; s=meesny; d=iki.fi; t=1670077217; a=rsa-sha256; cv=none; b=M+Oszv9Z+lbVEV47LTlaJT1YEkH45nMzhL/JnpGBes9b8YU7PVn3ym0XqelLmzBg1UIn3G j4hKZSVX7LNzHbMkWcXbxBIfCF48im9y4ZcGjHjjcW7GGXXSTEGmNkUWNayraOKmm++RaA bMZuEA6h0qCdQvnv2yxP3ZMY1jx3pYw= ARC-Authentication-Results: i=1; ORIGINATING; auth=pass smtp.auth=msmakela smtp.mailfrom=marko.makela@iki.fi Date: Sat, 3 Dec 2022 16:20:16 +0200 From: Marko =?iso-8859-1?q?M=E4kel=E4?= To: VDR Mailing List Message-ID: References: <5361cb76-ff2e-f50c-7f10-b64e402f0012@tvdr.de> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <5361cb76-ff2e-f50c-7f10-b64e402f0012@tvdr.de> X-LSpam-Score: -1.1 (-) X-LSpam-Report: No, score=-1.1 required=5.0 tests=BAYES_00=-1.9, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, DKIM_VALID_EF=-0.1, FROM_EXCESS_BASE64=0.979 autolearn=ham autolearn_force=no Subject: [vdr] [PATCH] Race conditions on shutdown X-BeenThere: vdr@linuxtv.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: VDR Mailing List List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Reply-To: VDR Mailing List Errors-To: vdr-bounces@linuxtv.org Sender: "vdr" 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;