From patchwork Mon May 20 11:40:11 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Florian Weimer X-Patchwork-Id: 1101950 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=pass (mailfrom) smtp.mailfrom=sourceware.org (client-ip=209.132.180.131; helo=sourceware.org; envelope-from=libc-alpha-return-102100-incoming=patchwork.ozlabs.org@sourceware.org; receiver=) Authentication-Results: ozlabs.org; dmarc=fail (p=none dis=none) header.from=redhat.com Authentication-Results: ozlabs.org; dkim=pass (1024-bit key; secure) header.d=sourceware.org header.i=@sourceware.org header.b="iiAcLPNE"; dkim-atps=neutral Received: from sourceware.org (server1.sourceware.org [209.132.180.131]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 456xlp4kJRz9s9N for ; Mon, 20 May 2019 21:40:22 +1000 (AEST) DomainKey-Signature: a=rsa-sha1; c=nofws; d=sourceware.org; h=list-id :list-unsubscribe:list-subscribe:list-archive:list-post :list-help:sender:from:to:subject:date:message-id:mime-version :content-type; q=dns; s=default; b=JWlEZYjjYQwe4f0srx62QmISq7JHD TRtNnknPRaSKfAw+KDTHHSplyCn/sMNCpJA2r8WoEfZ7g/sIJQN9dFBqeLJQWlE2 crG8voCD4mRAvk3cb3LSI+T49xP5Rs968PuYfi8QpmMKEOVHWZVCNsrqHy1vVbOn pkHSshCJKx3/6E= DKIM-Signature: v=1; a=rsa-sha1; c=relaxed; d=sourceware.org; h=list-id :list-unsubscribe:list-subscribe:list-archive:list-post :list-help:sender:from:to:subject:date:message-id:mime-version :content-type; s=default; bh=rxmkGVvSJTVwjOeKDzULhO9B4NI=; b=iiA cLPNEYTlhmOgwN2wkIQPAp1eAeVljfmwEzPls24dEZeIF/hUDmeSkxB+psBVbIBs up4Bv6He1uYWkSESGI0dOWhxcF7v/PYOBu7nOGvQ/uQCwrHaGbVhk7u3SoHeoKke GSr0TnlREsddGc1wGtAe6xOigcv4it/1JJ0M1Obs= Received: (qmail 70302 invoked by alias); 20 May 2019 11:40:15 -0000 Mailing-List: contact libc-alpha-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: libc-alpha-owner@sourceware.org Delivered-To: mailing list libc-alpha@sourceware.org Received: (qmail 70148 invoked by uid 89); 20 May 2019 11:40:15 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-19.1 required=5.0 tests=AWL, BAYES_00, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, SPF_HELO_PASS autolearn=ham version=3.3.1 spammy=H*r:sk:libc-al, lock X-HELO: mx1.redhat.com From: Florian Weimer To: libc-alpha@sourceware.org Subject: [PATCH] wcsmbs: Fix data race in __wcsmbs_clone_conv [BZ #24584] Date: Mon, 20 May 2019 13:40:11 +0200 Message-ID: <874l5pl5gk.fsf@oldenburg2.str.redhat.com> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/26.1 (gnu/linux) MIME-Version: 1.0 This also adds an overflow check and documents the synchronization requirement in . 2019-05-20 Florian Weimer [BZ #24584] * wcsmbs/wcsmbsload.c (__wcsmbs_clone_conv): Acquire __gconv_lock before updating __counter field and release it afterwards. Add overflow check. * iconv/gconv_int.h (struct __gconv_loaded_object): Mention synchronization requirement for __counter member. diff --git a/iconv/gconv_int.h b/iconv/gconv_int.h index ea41d6feaa..9510102c07 100644 --- a/iconv/gconv_int.h +++ b/iconv/gconv_int.h @@ -45,7 +45,8 @@ struct __gconv_loaded_object const char *name; /* Reference counter for the db functionality. If no conversion is - needed we unload the db library. */ + needed we unload the db library. __gconv_lock is used to + synchronize updates to this field. */ int counter; /* The handle for the shared object. */ diff --git a/wcsmbs/wcsmbsload.c b/wcsmbs/wcsmbsload.c index 5494d0a23e..e33a9c1312 100644 --- a/wcsmbs/wcsmbsload.c +++ b/wcsmbs/wcsmbsload.c @@ -20,6 +20,7 @@ #include #include #include +#include #include #include @@ -223,12 +224,24 @@ __wcsmbs_clone_conv (struct gconv_fcts *copy) /* Copy the data. */ *copy = *orig; - /* Now increment the usage counters. - Note: This assumes copy->*_nsteps == 1. */ + /* Now increment the usage counters. Note: This assumes + copy->*_nsteps == 1. The current locale holds a reference, so it + is still there after acquiring the lock. */ + + __libc_lock_lock (__gconv_lock); + + bool overflow = false; if (copy->towc->__shlib_handle != NULL) - ++copy->towc->__counter; + overflow |= __builtin_add_overflow (copy->towc->__counter, 1, + ©->towc->__counter); if (copy->tomb->__shlib_handle != NULL) - ++copy->tomb->__counter; + overflow |= __builtin_add_overflow (copy->tomb->__counter, 1, + ©->tomb->__counter); + if (overflow) + __libc_fatal ("\ +Fatal glibc error: gconv module reference counter overflow\n"); + + __libc_lock_unlock (__gconv_lock); }