From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mx0a-0010f301.pphosted.com (mx0a-0010f301.pphosted.com [148.163.149.254]) by sourceware.org (Postfix) with ESMTPS id 49E793857724 for ; Tue, 17 Oct 2023 19:14:47 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 49E793857724 Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=rice.edu Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=rice.edu ARC-Filter: OpenARC Filter v1.0.0 sourceware.org 49E793857724 Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=148.163.149.254 ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1697570088; cv=none; b=foH/PHYXLKhtgDYh5Chf8s8SRrQsNq2gs0DqDs2rEm8z29kpPbCNBVqZTRwR5K0gt6UptvwfOo0/t33zePbTnYr00KoAxZbUyt5tUQjZIkgzRKO0cjG2dsexnY6o7kErkxfm3whOSHqnt2/6yxLxvvOrvLjexs2wIRrA1UKWcew= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1697570088; c=relaxed/simple; bh=+yZGjkwyrH88RLYL+t/ajTpPiDEOi3N4OxpPLqh9Ryw=; h=DKIM-Signature:MIME-Version:From:Date:Message-ID:Subject:To; b=xzi6D7dRMmxltCkitKWcLcqERaRgLxQCnNoEQuo0foRrGUokAuE+0GDZqKFg0xlhZjBUzswbLfx09WwnAepfi/eOZ8BTWm7gRmVkXGRNdWk1U5ViM/wBB8jgqwBFTxchGEt0CkWEyX1Uq9Z5Gnownt6dtC3RBtlRRHglTiE9shM= ARC-Authentication-Results: i=1; server2.sourceware.org Received: from pps.filterd (m0102855.ppops.net [127.0.0.1]) by mx0b-0010f301.pphosted.com (8.17.1.22/8.17.1.22) with ESMTP id 39HHP1hi006805 for ; Tue, 17 Oct 2023 14:14:46 -0500 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=rice.edu; h= mime-version:references:in-reply-to:from:date:message-id:subject :to:cc:content-type; s=ricemail; bh=LQX9m+WiYqmVZPAeCxMSPXyOBOdW 5LCT1z6GfxDUqZs=; b=IMpoOOP1BwzTy0eMzFWScYq2vhKoiyaGuLyxK4QGKIKK DhOowhqr1ue/1+DLGAviKliwu/Qp5bwCm0DEnBHGZfrbhk4/UNaoCDGTt2xytGKw HkyYfUOpj8PMAh7hYu49d2IfFTjwFdAhMN3z6ROjNasL7Jx00XCsluafr1GyRcuf SJuGIiWsRwEyIPwNo/WE4rNjZ6XOz54gj8LwZfX4Lpp7vOSZq8MplW5Ko+lHCPhU TA7dZlKYbiM6rH0BUixFDqdKgzI9cUVdqePX87LTHMOkCOImCPOy/IbCedUS9tpd eThDqTrS0qFZtWlJW1Y39CciHnL3Y/8SgP9gpsAR7g== Received: from mail-wr1-f72.google.com (mail-wr1-f72.google.com [209.85.221.72]) by mx0b-0010f301.pphosted.com (PPS) with ESMTPS id 3tsc869vnk-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128 verify=NOT) for ; Tue, 17 Oct 2023 14:14:46 -0500 (CDT) Received: by mail-wr1-f72.google.com with SMTP id ffacd0b85a97d-32d9cd6eb0bso2632274f8f.0 for ; Tue, 17 Oct 2023 12:14:46 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1697570084; x=1698174884; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=LQX9m+WiYqmVZPAeCxMSPXyOBOdW5LCT1z6GfxDUqZs=; b=XFkOSa9k6nqlk7f336b509qzM93GsczOUelpIPeWkIW2gUTllot8u5LdcTqquPstWV 60Ymr6gA0cd0QAldllHQQNC0989S4SpF+mgrB9k2VVa4EINL8wztQ6Sz2lPmGtlARl/1 9Ogw85TA0Hkd+AxehKi0H7BaqDs5nwLpQz0bs2KpP+xUYKyc9me2Upe3XYBWNKY8y7MO NJeUlR+yDwhV0vmYGlx+zq0XqyA2ArZ9JUV6b2x/oYxBAZQk8K7xMpJMMbgr5Syb2HOU yAyhoIaT+sk1RpqvKhhQs3z/p2QLzl2eUqIpcoG/mLGVM+gHhPMsmGRppnpUN5+/L7TB h4eQ== X-Gm-Message-State: AOJu0YwYyHvZI/C5UkkI8SZUXiaNhmYXEpf+SBFKDza9ESbm9CSTB9BU 5w9bHgaDNeewnezdSLOGn7AQi2/k0jv6MsRkaDISCv+FIEpi2UA4aUzR2Usq5samJmhOcMXMy9i +2anJwyLYCxDUO/GI7b6YbEWIVPm+D0UvwwvSKPCWrIP1rahbEVSQXIFn3S5NrbqH8imh1yMtBc X4ONA= X-Received: by 2002:a5d:58da:0:b0:32d:b051:9a2b with SMTP id o26-20020a5d58da000000b0032db0519a2bmr2665178wrf.2.1697570084209; Tue, 17 Oct 2023 12:14:44 -0700 (PDT) X-Google-Smtp-Source: AGHT+IFMVxVZ6mHNvnDOlrlPX4eZGYCuyWn9INLD509bJZoMcCwVend6ZmWebLGiu+8HmAU5p/NLJeKFixDCVgeJq+c= X-Received: by 2002:a5d:58da:0:b0:32d:b051:9a2b with SMTP id o26-20020a5d58da000000b0032db0519a2bmr2665170wrf.2.1697570083936; Tue, 17 Oct 2023 12:14:43 -0700 (PDT) MIME-Version: 1.0 References: <301fac87e83ebbbd677750579ae9a3429b461bdf.camel@klomp.org> <20231010134300.53830-1-mark@klomp.org> <20231010134300.53830-4-mark@klomp.org> In-Reply-To: From: Heather McIntyre Date: Tue, 17 Oct 2023 14:14:33 -0500 Message-ID: Subject: Re: [PATCH 04/16] libelf: Fix deadlock in elf_cntl To: Mark Wielaard Cc: elfutils-devel@sourceware.org Content-Type: multipart/alternative; boundary="000000000000d5b0f20607ee5558" X-Proofpoint-DLP: Gmail-Outbound X-Proofpoint-Virus-Version: vendor=baseguard engine=ICAP:2.0.272,Aquarius:18.0.980,Hydra:6.0.619,FMLib:17.11.176.26 definitions=2023-10-17_03,2023-10-17_01,2023-05-22_02 X-Spam-Status: No, score=-10.8 required=5.0 tests=BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,GIT_PATCH_0,HTML_MESSAGE,SPF_HELO_NONE,SPF_NONE,TXREP autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org List-Id: --000000000000d5b0f20607ee5558 Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable You are right. I changed the code to just rely on if (__libelf_readall (elf) =3D=3D NULL) and this seems to work just fine. On Tue, Oct 10, 2023 at 10:23=E2=80=AFAM Mark Wielaard wro= te: > Hi Heather, > > On Tue, 2023-10-10 at 15:42 +0200, Mark Wielaard wrote: > > From: Heather McIntyre > > > > * libelf/elf_cntl.c (elf_cntl): Move rwlock_wrlock, rwlock_unlock, > > inside case switch statements. > > > > Signed-off-by: Heather S. McIntyre > > Signed-off-by: Mark Wielaard > > --- > > libelf/elf_cntl.c | 11 +++++++---- > > 1 file changed, 7 insertions(+), 4 deletions(-) > > > > diff --git a/libelf/elf_cntl.c b/libelf/elf_cntl.c > > index 04aa9132..64087c7d 100644 > > --- a/libelf/elf_cntl.c > > +++ b/libelf/elf_cntl.c > > @@ -48,13 +48,16 @@ elf_cntl (Elf *elf, Elf_Cmd cmd) > > return -1; > > } > > > > - rwlock_wrlock (elf->lock); > > + > > > > switch (cmd) > > { > > case ELF_C_FDREAD: > > + rwlock_rdlock (elf->lock); > > + int addr_isnull =3D elf->map_address =3D=3D NULL; > > + rwlock_unlock(elf->lock); > > /* If not all of the file is in the memory read it now. */ > > - if (elf->map_address =3D=3D NULL && __libelf_readall (elf) =3D= =3D NULL) > > + if (addr_isnull && __libelf_readall (elf) =3D=3D NULL) > > { > > /* We were not able to read everything. */ > > result =3D -1; > > Can't we just rely on if (__libelf_readall (elf) =3D=3D NULL)? > > __libelf_readall already does locking and will return non-NULL if elf- > >map_address is already set. So it looks like the extra check (and > locking) to check addr_isnull is redundant and just make the code more > complex. > > > @@ -64,7 +67,9 @@ elf_cntl (Elf *elf, Elf_Cmd cmd) > > > > case ELF_C_FDDONE: > > /* Mark the file descriptor as not usable. */ > > + rwlock_wrlock (elf->lock); > > elf->fildes =3D -1; > > + rwlock_unlock (elf->lock); > > break; > > > > default: > > This looks correct. All other accesses to elf->fildes seem to be done > under the elf->lock too. > > > @@ -73,7 +78,5 @@ elf_cntl (Elf *elf, Elf_Cmd cmd) > > break; > > } > > > > - rwlock_unlock (elf->lock); > > - > > return result; > > } > > --000000000000d5b0f20607ee5558--