From patchwork Thu Mar 11 09:42:20 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Tobias Burnus X-Patchwork-Id: 1451028 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=gcc.gnu.org (client-ip=2620:52:3:1:0:246e:9693:128c; helo=sourceware.org; envelope-from=gcc-patches-bounces@gcc.gnu.org; receiver=) Received: from sourceware.org (server2.sourceware.org [IPv6:2620:52:3:1:0:246e:9693:128c]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 4Dx3rx2JrRz9sR4 for ; Thu, 11 Mar 2021 20:42:39 +1100 (AEDT) Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 14AFD3836C4D; Thu, 11 Mar 2021 09:42:34 +0000 (GMT) X-Original-To: gcc-patches@gcc.gnu.org Delivered-To: gcc-patches@gcc.gnu.org Received: from mx-relay77-hz2.antispameurope.com (mx-relay77-hz2.antispameurope.com [94.100.136.177]) by sourceware.org (Postfix) with ESMTPS id AA8353874C2E for ; Thu, 11 Mar 2021 09:42:30 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org AA8353874C2E Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=net-b.de Authentication-Results: sourceware.org; spf=none smtp.mailfrom=prvs=06977f9bf4=burnus@net-b.de Received: from s041.wsp.plusnet.de ([195.90.7.81]) by mx-relay77-hz2.antispameurope.com; Thu, 11 Mar 2021 10:42:26 +0100 Received: from [192.168.0.28] (port-92-195-197-236.dynamic.as20676.net [92.195.197.236]) by s041.wsp.plusnet.de (Postfix) with ESMTPSA id 2FC062C0350; Thu, 11 Mar 2021 10:42:22 +0100 (CET) To: gcc-patches , fortran , Jerry DeLisle , mscfd@gmx.net From: Tobias Burnus Subject: [Patch] Fortran: Fix libgfortran I/O race with newunit_free [PR99529] Message-ID: Date: Thu, 11 Mar 2021 10:42:20 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.8.0 MIME-Version: 1.0 Content-Language: en-US X-cloud-security-sender: burnus@net-b.de X-cloud-security-recipient: gcc-patches@gcc.gnu.org X-cloud-security-Virusscan: CLEAN X-cloud-security-disclaimer: This E-Mail was scanned by E-Mailservice on mx-relay77-hz2.antispameurope.com with B304344AF9D X-cloud-security-connect: s041.wsp.plusnet.de[195.90.7.81], TLS=1, IP=195.90.7.81 X-cloud-security-Digest: 43a39f3f7329755c4de017b71c5cbaf8 X-cloud-security: scantime:1.346 X-Spam-Status: No, score=-13.6 required=5.0 tests=BAYES_00, GIT_PATCH_0, KAM_DMARC_STATUS, KAM_LAZY_DOMAIN_SECURITY, RCVD_IN_DNSWL_LOW, RCVD_IN_MSPIKE_H3, RCVD_IN_MSPIKE_WL, SPF_HELO_NONE, SPF_NONE, TXREP autolearn=ham autolearn_force=no version=3.4.2 X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on server2.sourceware.org X-BeenThere: gcc-patches@gcc.gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gcc-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: gcc-patches-bounces@gcc.gnu.org Sender: "Gcc-patches" Hi all, as found by Martin (thanks!) there is a race for newunit_free. While that call is within the unitlock for the calls in io/unit.c, the call in transfer.c did not use locks. Additionally, unit = get_gfc_unit (dtp->common.unit, do_create); set_internal_unit (dtp, unit, kind); gets first the unit (with proper locking when using the unit number dtp->common.unit) but then in set_internal_unit it re-sets the unit number to the same number without locking. That causes race warnings and if the assignment is not atomic it is a true race. OK for mainline? What about GCC 10? As Martin notes in the email thread and in the PR there are more race warnings (and likely true race issues). Tobias Fortran: Fix libgfortran I/O race with newunit_free [PR99529] libgfortran/ChangeLog: * io/transfer.c (st_read_done_worker, st_write_done_worker): Add unit_lock lock/unlock around newunit_free call. * io/unit.c (set_internal_unit): Don't reset the unit_number to the same number as this cause race warnings. diff --git a/libgfortran/io/transfer.c b/libgfortran/io/transfer.c index 27bee9d4e01..a6f115d5803 100644 --- a/libgfortran/io/transfer.c +++ b/libgfortran/io/transfer.c @@ -4358,7 +4358,9 @@ st_read_done_worker (st_parameter_dt *dtp) free (dtp->u.p.current_unit->ls); dtp->u.p.current_unit->ls = NULL; } + LOCK (&unit_lock); newunit_free (dtp->common.unit); + UNLOCK (&unit_lock); } if (dtp->u.p.unit_is_internal || dtp->u.p.format_not_saved) { @@ -4446,7 +4448,9 @@ st_write_done_worker (st_parameter_dt *dtp) free (dtp->u.p.current_unit->ls); dtp->u.p.current_unit->ls = NULL; } + LOCK (&unit_lock); newunit_free (dtp->common.unit); + UNLOCK (&unit_lock); } if (dtp->u.p.unit_is_internal || dtp->u.p.format_not_saved) { diff --git a/libgfortran/io/unit.c b/libgfortran/io/unit.c index 2ea664331bc..b0cc6ab2301 100644 --- a/libgfortran/io/unit.c +++ b/libgfortran/io/unit.c @@ -456,7 +456,6 @@ set_internal_unit (st_parameter_dt *dtp, gfc_unit *iunit, int kind) { gfc_offset start_record = 0; - iunit->unit_number = dtp->common.unit; iunit->recl = dtp->internal_unit_len; iunit->internal_unit = dtp->internal_unit; iunit->internal_unit_len = dtp->internal_unit_len;