From patchwork Fri Jan 6 15:47:18 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Cupertino Miranda X-Patchwork-Id: 62804 X-Patchwork-Delegate: fweimer@redhat.com Return-Path: X-Original-To: patchwork@sourceware.org Delivered-To: patchwork@sourceware.org Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 894123858CDA for ; Fri, 6 Jan 2023 15:47:50 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 894123858CDA DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=sourceware.org; s=default; t=1673020070; bh=8zvNX9KKdwOdf/8WExkCGdzOdwvkkF4N2eTLBsuzyjY=; h=To:Cc:Subject:Date:List-Id:List-Unsubscribe:List-Archive: List-Post:List-Help:List-Subscribe:From:Reply-To:From; b=CVv3zZd+kGKi1Dy7gIQU41GgYSexhIjhdf8ARlJRloeT3zl+sin+/EyMWOeZy6vMG KMqt1aLvfHeXUdWPyS7PlF1NJTMOv3VsnmR0pjqjj+nclFsG6RcJDY35CBInittt8t saTClM/D8zPstxosOmt3NOZNi4GPkrM0xlS8NpKk= X-Original-To: libc-alpha@sourceware.org Delivered-To: libc-alpha@sourceware.org Received: from mx0a-00069f02.pphosted.com (mx0a-00069f02.pphosted.com [205.220.165.32]) by sourceware.org (Postfix) with ESMTPS id A22043858D28 for ; Fri, 6 Jan 2023 15:47:28 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org A22043858D28 Received: from pps.filterd (m0246627.ppops.net [127.0.0.1]) by mx0b-00069f02.pphosted.com (8.17.1.19/8.17.1.19) with ESMTP id 306APOCb017699 for ; Fri, 6 Jan 2023 15:47:27 GMT Received: from iadpaimrmta01.imrmtpd1.prodappiadaev1.oraclevcn.com (iadpaimrmta01.appoci.oracle.com [130.35.100.223]) by mx0b-00069f02.pphosted.com (PPS) with ESMTPS id 3mtbp13edb-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=OK) for ; Fri, 06 Jan 2023 15:47:27 +0000 Received: from pps.filterd (iadpaimrmta01.imrmtpd1.prodappiadaev1.oraclevcn.com [127.0.0.1]) by iadpaimrmta01.imrmtpd1.prodappiadaev1.oraclevcn.com (8.17.1.5/8.17.1.5) with ESMTP id 306EbuQQ032592 for ; Fri, 6 Jan 2023 15:47:26 GMT Received: from nam10-mw2-obe.outbound.protection.outlook.com (mail-mw2nam10lp2102.outbound.protection.outlook.com [104.47.55.102]) by iadpaimrmta01.imrmtpd1.prodappiadaev1.oraclevcn.com (PPS) with ESMTPS id 3mwft178uj-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=OK) for ; Fri, 06 Jan 2023 15:47:26 +0000 ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=DdHiWiEd8nLvUi3JXmpLVkQ6hVcFds3tYB6Jik3MgkF9D7dEs/QGxiJKrip+fyhYnKinty5yaDLT7OEqthyBgSE/8vZLer0Uc4w78s00rOuuDUa2ogEhQL5mqp6C45v9FzFJAGY2lBw0PhvITJ2kPBl9FZZhGFZYXLQJ2jnT4YmVQED8KIogxZHZBp0MVYirPkraVMLGTqSEjtRqkFW7TGcB0zIfQrUOpCzFfQRYfVCZtuHU+or0iCzvGP44wjwjYtE7/1rFwXqfU+NKw4ifCjTThNv1eNwOt2cfYYBwJZLO/5DbXvcFEi1DJpVR7XdJFFEPNHzsceXf37YQYw7bTQ== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-AntiSpam-MessageData-ChunkCount:X-MS-Exchange-AntiSpam-MessageData-0:X-MS-Exchange-AntiSpam-MessageData-1; bh=8zvNX9KKdwOdf/8WExkCGdzOdwvkkF4N2eTLBsuzyjY=; b=eCmdz/4z+UjNrc/+5XqPEzwmNFyzD12m+aiyhH58uZEgiIsvDX/Bs9QdmekmDYQIxEzxFUmMXGll7dTSos94OdlKSwXUMG2vYQngiURCVqHZp7vyrjiP3/WAsNtgYvHcK3EAHMZNr38HeWrNJc1cvwKoN4M/SN48AmNkhbjABvaRN281mXuSrrP+TuaH2/bqeWAMvjeVWSCvhTsG+oqTHvCCvqBak2qty4AJUEIZUit/o/+/lgv0v5DuxJqaJlvuY9b7AQbJUY1uVVd/aZAc+XRl9owt7Y8iCcn4wjyWl0nFmRLW9zOGRkWTCnZ9WCva5Y+zcG44j3ycxv6l2Y5s4Q== ARC-Authentication-Results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=oracle.com; dmarc=pass action=none header.from=oracle.com; dkim=pass header.d=oracle.com; arc=none Received: from BN6PR1001MB2340.namprd10.prod.outlook.com (2603:10b6:405:30::36) by MN2PR10MB4288.namprd10.prod.outlook.com (2603:10b6:208:1dc::7) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.5944.19; Fri, 6 Jan 2023 15:47:23 +0000 Received: from BN6PR1001MB2340.namprd10.prod.outlook.com ([fe80::8681:5931:e558:2638]) by BN6PR1001MB2340.namprd10.prod.outlook.com ([fe80::8681:5931:e558:2638%7]) with mapi id 15.20.5944.019; Fri, 6 Jan 2023 15:47:23 +0000 User-agent: mu4e 1.4.15; emacs 28.1 To: libc-alpha@sourceware.org Cc: "Jose E. Marchesi" , Elena Zannoni Subject: [PING] [PATCH v2] Resolve-flockfile-funlockfile-differences Date: Fri, 06 Jan 2023 15:47:18 +0000 Message-ID: <87ilhj3bsp.fsf@oracle.com> X-ClientProxiedBy: AM4PR0202CA0004.eurprd02.prod.outlook.com (2603:10a6:200:89::14) To BN6PR1001MB2340.namprd10.prod.outlook.com (2603:10b6:405:30::36) MIME-Version: 1.0 X-MS-PublicTrafficType: Email X-MS-TrafficTypeDiagnostic: BN6PR1001MB2340:EE_|MN2PR10MB4288:EE_ X-MS-Office365-Filtering-Correlation-Id: 27bb0df8-4e1e-485b-2d56-08daeffd4d92 X-MS-Exchange-SenderADCheck: 1 X-MS-Exchange-AntiSpam-Relay: 0 X-Microsoft-Antispam: BCL:0; X-Microsoft-Antispam-Message-Info: zvvDIRLv5AHQjxNDSjsAF05ARzSSFe3IzAAe89ZfCZr6U6f7z5xaQ1ckYE+bEAsd6dmv3lJesIM3tnlLzVSBwAgJMLbLIUSrVrc7pn5nhv4iORGuiFSLn5oY6zp+xfQmGNAWEWe6RlbWgPzZio1iNaklkXkyc3VyNkOK1aAkWS5/tnqcLEV31o+g18Dk3HgS3v32ib37nJthiT7+Pwwodp44C70eYC6fgPWdTCQ4qCYJ1Bi1wysacAYV1dKzNXjDxAzq0Q/UVduLAExQtcHGAaXhYVXCkJ5zUdO4fJWDyi8nsba/AlUBIkemPkWSCE4+jGImkf4alLO71Z723iDxUapxJeUmJk8CyAeeZDIv3eESzVx9iUG4/UVYrSsQPMtYneTtsbewFG5UC7WJE3vW/VtU8Nx+i1EcuV8ug7QkKeZ64Hcv6vRmFIW3vu6b0Op59xhonD3O7U1O3DfaW/Uu+ZJQDj9ue44Ulr2a/BvYM2wGema8xbcWMEs3VUz7g0OBSjG8Qu18zhkKyTKE8iFgK7rhvXVYp17CjmaI3aaN77SoQ+3w21CLeZCZ/UF4jfttz4Wnu1xGnUizGr0t3Zy4mz/Ybe258+Mxj8sVL9d+Jd4LXxHBPt2MXaeDuUp4lvT9CpLKDp2ZKpwVH9MGXz5FOw== X-Forefront-Antispam-Report: CIP:255.255.255.255; CTRY:; LANG:en; SCL:1; SRV:; IPV:NLI; SFV:NSPM; H:BN6PR1001MB2340.namprd10.prod.outlook.com; PTR:; CAT:NONE; SFS:(13230022)(346002)(136003)(39860400002)(376002)(396003)(366004)(451199015)(66556008)(66946007)(4326008)(66476007)(41300700001)(54906003)(6916009)(5660300002)(8676002)(44832011)(8936002)(2906002)(316002)(6486002)(53546011)(107886003)(6666004)(6506007)(478600001)(86362001)(83380400001)(186003)(6512007)(36756003)(2616005)(38100700002); DIR:OUT; SFP:1101; X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1 X-MS-Exchange-AntiSpam-MessageData-0: rcudVrxbS/rBBTyU1u8Q3hdK32SJUO2MEv18MD0YvpfrKPi1AC46ae9f73WtwJ7uQ+IZCq08AswdqWbraJpYt81GJ4Hm/OEGUq6zOboQALeSaHQT1jPu9TJS8haSYc8o0mqxEzAPmZ1Cf/Gb32ej/Z+GY7mi1p8UUSh3FlxMUy7XHHB1x6iSbQ0qMqCUq3KF/6yH9Nuf3TqPeP7pb8Z3Yz2ZefGeu8DJrxtsa8ExYwTSW940tQQ15qUw+Pj93jIsyOrGQdt2jrvTNnrK7gslv584GmlYq7hH/yQCnTdG5A7R631kTG4E+vo9hLcr2PJdPnMWCP2niriJomDnnWKKEwCvp/1PCexh9xgGQgOalkESOdxYaiy1Ooa2PRqXOmIxFg9kQGpkbtL2YyT/VJuTloGz6R6yW2FI5/LV67u9jMkyYDbd/bHWTredJGaF84ych6jtWR3vBp91DW7teB7AItAxBUf2WACF9EyrpfXz8DBXav0KUPjPZelZ2jKQWC/skGh7TU2ciknASZG79Bw9qMFiU+7LuzLOqp4rhZIdDy6kQyn4QScRuvFIH8frMvYJogsp29aXtbuZinGuGVap9H+pJVE6XkXXZx4r4vww5zyTdyyNw9FJ9DIUgxrCIXfj9EfMsPcd7FTBtpaHStw006lA7WXOhG8q7+sILJ+FC74MJrWBSssxxp+yGr+yoXoC9P6t0eNvH/bDmg3iI+nm9m/crUbAkKirdrZ8ZaOfb7i7mOuiWIYxhVCbXwTPl/QPDlhFdLbDs1Afyv7ukpj2fVZwYkrWRp9fMiNIOyup46iWE58fdEUquuTnkra5piL5tIcfygkgb8kqZcTjDk4HJ2AMul9+PC9AaHFerPlgl3GvVWJpzT8IcGy+uBDDhclu1g+m4NZTUp/dNqc6pzJn60cR3fRp31VcReenHaO8NtgvUkaJvB/wDx2el/QTNf8Kq+PIMowp4jenFsrFeYmUq89DNL4Bp1U2W0U2hgB+yVS44S4sz+dlCHAgkOqiPibM4b0VAEYb9J15crncqCi+v+BHIHBfzM/Lf9QC66NJzULeKaP9e39wXJklOGc/Vr99ITdnGbzX/K4HQ7j38lEUxvyMesmn6qcWqa83dgbVzwTqekNXUmqbmeG0AVWC1tiWZBH7F0QS6/5lOVIZ80NDjR/miuYVjaRyEMh+XyXK9PeUK7lzRILuGawEEZqAXlHhk6Wi1yUONU8/4Z3s3uayOHBqYRXud04GFS0RVTSVSUrjfbhJKf6mMYC4mixlduLK/1zgh+nghr0IwszjusSER1Pbdn5xhZmv3XwmM3GpWMmfFWyD5ayPj25ggmSb1t0lt5bwcXYVjY9bg1joPNwLx7kgArmrABMEQ3MlmzbDiflWKj8D+r5Jdb2DnZ7u0DaaPQzIKNnOyTPijv+F5/iNtFZu4WlO+3u0zQGJd10i15tX/HSd1HvPtVu3/8aTXFzMDiybciMD+BI2wQrzhB67yNHd1JKswr9zeKkLgCQmTorvk3jrO6RDEjwSGXZmUW34Eq+1rD1c+XpSd7ddwVyR9t6n2mkuA28+GdB2gdzN/NYSBB4iNMcTUDXpAuRzGNhQbMdPSre+11idYi4b99S3nciOqsoorbjHPvbsY3neRQw= X-MS-Exchange-AntiSpam-ExternalHop-MessageData-ChunkCount: 1 X-MS-Exchange-AntiSpam-ExternalHop-MessageData-0: 7WbAli/NISnYF1MIIgOEVZXWFbPwrYk1bOTTDegItiYKHHNth4FY544fw+Hsn9phA5KHMsoKRXMaj7q1tBDIZLzuNG6PsyNHze2hvuR+kjlVYnhs9WRCJkNb34/VCMZCzWQXW4oPtM3gWq3NoCfcwfXCbkdhVeElO9L7FGcLkpJ4NTj/sQl4wxZnjn0NPJ7T6czst+efM+1hNB3/1onlrmIj58eyZiMU0IAPooC00h2JpNvDZfcElTKpQXHm4zgT+2O23/KhwQmVv6HeGJFWPq68eZ4fwDwuBi7aP9cwQCf7krL0r+X5oQ7yKMm9ooo/1Nxcm0KwikOJ44O62MCc/2Ty2f/t7gS5YQs8XOv1joE/CvvSblueC05/W5Bjnv8oNtoFk2JdYX9vqrpU0Cz2jhHBHQSjiqyqw5GKpiFj24/y+631MaHJw66JVnPot9SEYe4ZBBumYEQoBnuTCJNXZh5ZG2YcJIsD37YryggyVoPUJGMp/FLzysWKewSmG7s3hiqII3hOPBid8H3RyKqisFLX6hTdXHgwB++z37gvaX3r5g08fk1OQLoqy39zhA2gE4gqIFZPaXQoBb/UIe03xySYyKbyV/X5pQIjHJF4PL1C6z0mI9fadNDa4w5uXhnrmHTYTiaFYCNtywIs4Akrtr/k77liwQ8B6JxC8u4Nsn09LkEnWzGI2cb84pXgyGQc77KHz/a2Vq8bJ5tm0LfRlEOLVQ3EHOr38Rb2u9A/rI+XdoFvUNknvHbP3NHCFbwu X-OriginatorOrg: oracle.com X-MS-Exchange-CrossTenant-Network-Message-Id: 27bb0df8-4e1e-485b-2d56-08daeffd4d92 X-MS-Exchange-CrossTenant-AuthSource: BN6PR1001MB2340.namprd10.prod.outlook.com X-MS-Exchange-CrossTenant-AuthAs: Internal X-MS-Exchange-CrossTenant-OriginalArrivalTime: 06 Jan 2023 15:47:23.5879 (UTC) X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted X-MS-Exchange-CrossTenant-Id: 4e2c6054-71cb-48f1-bd6c-3a9705aca71b X-MS-Exchange-CrossTenant-MailboxType: HOSTED X-MS-Exchange-CrossTenant-UserPrincipalName: zhetmPOvJnkqLZ38M+L5O6XkqW1fKsQMjNTFAlhMBIZsWuD7L4JVZQUchLCd6zW28VBBRlHBsk14P6ZXuVqJYwMvMq4Kzk6TuDxEBcBWWa8= X-MS-Exchange-Transport-CrossTenantHeadersStamped: MN2PR10MB4288 X-Proofpoint-Virus-Version: vendor=baseguard engine=ICAP:2.0.205,Aquarius:18.0.923,Hydra:6.0.545,FMLib:17.11.122.1 definitions=2023-01-06_10,2023-01-06_01,2022-06-22_01 X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 mlxscore=0 mlxlogscore=999 adultscore=0 spamscore=0 malwarescore=0 bulkscore=0 phishscore=0 suspectscore=0 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2212070000 definitions=main-2301060121 X-Proofpoint-GUID: JsULiDQcjwtk5NhsBxFDVofUX_-3Uqbr X-Proofpoint-ORIG-GUID: JsULiDQcjwtk5NhsBxFDVofUX_-3Uqbr X-Spam-Status: No, score=-11.5 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, RCVD_IN_DNSWL_LOW, RCVD_IN_MSPIKE_H2, 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 X-BeenThere: libc-alpha@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Libc-alpha mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-Patchwork-Original-From: Cupertino Miranda via Libc-alpha From: Cupertino Miranda Reply-To: Cupertino Miranda Errors-To: libc-alpha-bounces+patchwork=sourceware.org@sourceware.org Sender: "Libc-alpha" This is a request for review of the patch sent by Patrick McGehearty based on an inconsistency in flockfile and funlockfile definitions (macro/function) when used in glibc code. At the same time this is a request for comments on the topic based on some personal concerns on the patch. After reading the description of the problem as explained in our bug tracking I decided to make a more targeted patch, which will also help to better explain the problem. Unfortunately at this stage I cannot functionally verify this patch due to how long it was identified and fixed at Oracle. The patch defines a local wrapper to flockfile and funlockfile such that both the function pointer passed to libc_cleanup_region_start and the direct call would point to the same definition. I have some concerns about the original presented patch as, IMO, it would allow any user level application to bypass the lock/unlock by directly writing to the file pointer flags, changing the original definition of f[unlock|lock]file as described in the man pages. Should both the macro/function definitions be coherent such that they should be used interchangeably in glibc code? Regards, Cupertino --- MORE TARGETED PATCH --- > From a0291671db0d92a8762de3a45fd322e471a19a24 Mon Sep 17 00:00:00 2001 > From: Patrick McGehearty > Date: Wed, 23 Nov 2022 21:02:02 +0000 > Subject: [PATCH v2] Resolve flockfile/funlockfile differences > > - - - - - - > Only difference from v1 is to correct indenting/tabs > for the changes to match other .c files in stdio-common. > - - - - - - > This patch resolves differences between flockfile/funlockfile and > LOCK_STREAM/UNLOCKSTREAM to avoid failure to unlock a stream mutex in > a multi-threaded application context which allows entering using > LOCK_STREAM but leaving using funlockfile or vice-versa. > This issue was detected during stress tests of a large proprietary > application. The cause and solution was identified by Gerd Rausch. > > The issue occurs because _IO_funlockfile has different definitions in > different contexts: > > Comparing the inline version in libio/libio.h > # define _IO_flockfile(_fp) \ > if (((_fp)->_flags & _IO_USER_LOCK) == 0) _IO_flockfile (_fp) > > And the non-inline version in stdio-common/flockfile.c > __flockfile (FILE *stream) > { > _IO_lock_lock (*stream->_lock); > } > > Note the lack of the _IO_USER_LOCK in the __flockfile version. This > difference means it is possible to bypass the lock in some cases and > not release the lock in other cases. Either way, it causes trouble. > > The proposed fix is to simple add the _IO_USER_LOCK guard to the > non-line versions of flockfile and funlockfile. > > Modified files: > stdio-common/flockfile.c > stdio-common/funlockfile.c > --- > stdio-common/flockfile.c | 3 ++- > stdio-common/funlockfile.c | 3 ++- > 2 files changed, 4 insertions(+), 2 deletions(-) > > diff --git a/stdio-common/flockfile.c b/stdio-common/flockfile.c > index 49f72c6..7c82847 100644 > --- a/stdio-common/flockfile.c > +++ b/stdio-common/flockfile.c > @@ -22,7 +22,8 @@ > void > __flockfile (FILE *stream) > { > - _IO_lock_lock (*stream->_lock); > + if ((stream->_flags & _IO_USER_LOCK) == 0) > + _IO_lock_lock (*stream->_lock); > } > weak_alias (__flockfile, flockfile); > weak_alias (__flockfile, _IO_flockfile) > diff --git a/stdio-common/funlockfile.c b/stdio-common/funlockfile.c > index bf44c99..b77b1b2 100644 > --- a/stdio-common/funlockfile.c > +++ b/stdio-common/funlockfile.c > @@ -23,7 +23,8 @@ > void > __funlockfile (FILE *stream) > { > - _IO_lock_unlock (*stream->_lock); > + if ((stream->_flags & _IO_USER_LOCK) == 0) > + _IO_lock_unlock (*stream->_lock); > } > weak_alias (__funlockfile, _IO_funlockfile) > weak_alias (__funlockfile, funlockfile); > -- > 1.8.3.1 diff --git a/stdio-common/vfscanf-internal.c b/stdio-common/vfscanf-internal.c index 2ad34050f3..5b55db962b 100644 --- a/stdio-common/vfscanf-internal.c +++ b/stdio-common/vfscanf-internal.c @@ -177,11 +177,15 @@ return EOF; \ } \ } while (0) + +static inline void checked_IO_funlockfile(FILE *s); +static inline void checked_IO_flockfile(FILE *s); + #define LOCK_STREAM(S) \ - __libc_cleanup_region_start (1, (void (*) (void *)) &_IO_funlockfile, (S)); \ - _IO_flockfile (S) + __libc_cleanup_region_start (1, (void (*) (void *)) &checked_IO_funlockfile, (S)); \ + checked_IO_flockfile (S) #define UNLOCK_STREAM(S) \ - _IO_funlockfile (S); \ + checked_IO_funlockfile (S); \ __libc_cleanup_region_end (0) struct ptrs_to_free @@ -197,6 +201,18 @@ struct char_buffer { struct scratch_buffer scratch; }; +static inline void +checked_IO_funlockfile(FILE *s) +{ + _IO_funlockfile (s); +} + +static inline void +checked_IO_flockfile(FILE *s) +{ + _IO_flockfile (s); +} + /* Returns a pointer to the first CHAR_T object in the buffer. Only valid if char_buffer_add (BUFFER, CH) has been called and char_buffer_error (BUFFER) is false. */