From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id EEEEBC433FE for ; Wed, 16 Nov 2022 05:33:46 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S229495AbiKPFdp (ORCPT ); Wed, 16 Nov 2022 00:33:45 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:52136 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229456AbiKPFdo (ORCPT ); Wed, 16 Nov 2022 00:33:44 -0500 Received: from mx0b-00082601.pphosted.com (mx0b-00082601.pphosted.com [67.231.153.30]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 4CF88263F for ; Tue, 15 Nov 2022 21:33:43 -0800 (PST) Received: from pps.filterd (m0109332.ppops.net [127.0.0.1]) by mx0a-00082601.pphosted.com (8.17.1.19/8.17.1.19) with ESMTP id 2AG2C8ZB009068; Tue, 15 Nov 2022 21:33:27 -0800 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=meta.com; h=message-id : date : subject : to : cc : references : from : in-reply-to : content-type : content-transfer-encoding : mime-version; s=s2048-2021-q4; bh=qh6Q+w2ZqvCiA54+3Enk73HTz6zaNwmAyiVhUVwZQaY=; b=EUh5hJFh7uOdsRnWPKkHoARglmO9ZEcnyTwsjyN/6EXe9QNaa0joKo9zquVxBhloWHzM v9FVSQV2+TEV6WaPopaiGWnOceB1VyQGEhACfIM/d6QEClByac569Yd8sDDA8BJm8IpZ fjGvhJz8L3Lt+ScJ4OQmdbI56Dc7e202gICWD8F3q/KqWO6rpT3m7ZzbZ7VvfK8fDRST TebUF2m18ZRriAI/JjyBn8DiYll5Q9lOXGFEeFoi3Ud8MlY1pnLFuOgki5EXcwfRKwEe EMWS9QEqPwXvjik5/xye8G3Fvj0H39XmwZluY4MPQuQi/z/Es2DXEEoTKuE2U5n3C36x hg== Received: from nam10-bn7-obe.outbound.protection.outlook.com (mail-bn7nam10lp2102.outbound.protection.outlook.com [104.47.70.102]) by mx0a-00082601.pphosted.com (PPS) with ESMTPS id 3kvpuws2fu-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Tue, 15 Nov 2022 21:33:27 -0800 ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=eztHEg6duYmDpYW5C2sWw51t6/XnyIWS9xsrr/dhg9B//OQhzbPsnZfdPh2GCJK3R5zktv3EEGdDnsJKpbzkSZP8IRnd7CeqPqHcbMt/BeWOeTD3tM1of6dekFL2fEmAeQzbETXMMCZKhSke2yBSTaCwUC9CVHrGy/14Ck/Wv03v+kPbfM3SJfhmNpLm5IvgcGZMcvTej4jamum8XLUb7a+mfk2P9TEDza5cWtMwv4m98RfhMnvdUfPvk5aGQjVYIS9g1Pd07ss84sl0M4VRrvN1OagpPSnEnyddy3kG6eAe4lT/K+uGo9lHqNbK3uvan87tcRNU2/X6T4rrUh1oJw== 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=qh6Q+w2ZqvCiA54+3Enk73HTz6zaNwmAyiVhUVwZQaY=; b=cM37NDLDqUD/DFsPcgtzNx/R2yeLMbZl/yZBUj/DqDiOb+B7HsIchY1d04PihJDnFcvqbUs/Gy5LHTu8j3tug9Toc4zbTpiKsVNYPdGbdr96ecp/xUBatfV5LQKRjy2bdT9v7GfF/rvHUYKIcoVgKv7WI5A63ogldvj4lJN6UqW6VysN5HjsYaRcF2mNrQ1Q1Sgj0p9wCe5l9xogMjSITlkT3bXkvuEZRaKPJu1zUcPXMWxkFEO67FFHH/jNX88FS72PD4POIbX3Ht6cFVKIqEoQsn0V0V9Mnw1PXXAfPoI1xduPKsOObdGaQK16059h41ft9+2BQl5qo7kRWq4Xgg== ARC-Authentication-Results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=meta.com; dmarc=pass action=none header.from=meta.com; dkim=pass header.d=meta.com; arc=none Received: from SN6PR1501MB2064.namprd15.prod.outlook.com (2603:10b6:805:d::27) by BN8PR15MB2786.namprd15.prod.outlook.com (2603:10b6:408:c3::23) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.5813.16; Wed, 16 Nov 2022 05:33:25 +0000 Received: from SN6PR1501MB2064.namprd15.prod.outlook.com ([fe80::ac66:fb37:a598:8519]) by SN6PR1501MB2064.namprd15.prod.outlook.com ([fe80::ac66:fb37:a598:8519%7]) with mapi id 15.20.5813.013; Wed, 16 Nov 2022 05:33:25 +0000 Message-ID: Date: Tue, 15 Nov 2022 21:33:21 -0800 User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:102.0) Gecko/20100101 Thunderbird/102.4.2 Subject: Re: [PATCH bpf-next v5 6/7] selftests/bpf: Add tests for bpf_rcu_read_lock() Content-Language: en-US To: Martin KaFai Lau , Yonghong Song Cc: Alexei Starovoitov , Andrii Nakryiko , Daniel Borkmann , kernel-team@fb.com, bpf@vger.kernel.org, Martin KaFai Lau References: <20221111165734.2524596-1-yhs@fb.com> <20221111165805.2528458-1-yhs@fb.com> <7bd272c8-6e28-cdcd-6728-a78a71f6b0d3@linux.dev> From: Yonghong Song In-Reply-To: <7bd272c8-6e28-cdcd-6728-a78a71f6b0d3@linux.dev> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit X-ClientProxiedBy: BY5PR17CA0019.namprd17.prod.outlook.com (2603:10b6:a03:1b8::32) To SN6PR1501MB2064.namprd15.prod.outlook.com (2603:10b6:805:d::27) MIME-Version: 1.0 X-MS-PublicTrafficType: Email X-MS-TrafficTypeDiagnostic: SN6PR1501MB2064:EE_|BN8PR15MB2786:EE_ X-MS-Office365-Filtering-Correlation-Id: 2a2aeb35-677f-42ed-6972-08dac794150c X-FB-Source: Internal X-MS-Exchange-SenderADCheck: 1 X-MS-Exchange-AntiSpam-Relay: 0 X-Microsoft-Antispam: BCL:0; X-Microsoft-Antispam-Message-Info: ZuF9/trg9LD8quGmagmsuyzzX8/Mv96iIR7CtSFKeCp6uiEi9XE0DNK4T4af+EphIVUhF9ElHvxT9FqQc7vGiHQEVbH5WcFa+PZo2anG3i7Ctr/UjwkmzJ+Tn+QYGqGGOr1bzWVXL5OLClL+gMDYy3BqeLnDWsBupqtVgjSk4ZOMPdgBw132y5V72h+0gnu7MKa+5bjzw6pepu6iJeKHkW7SoN8lnBmPgu2FU8vx4dpKtfiCGG+B7FTFWJdYTZTBRvgp8nCBcv0AbWo6aFOBr2wCVsEJC+iL5z4gW02osoem9pdxef1YV0Mco5/+UF4ZM4MbMmMmsDdRYk8XRgtyZgR6f1Oub9hwjMvsB+MBUMzlxgkgy1c6lx4HNBThckifR+JpXcQQ4Zs4WWJ1Kzv0V0muAVl3tiO6vKm406UKICCgVP27RQ50nbOVOIAxPcjAPPzXnMA9jzPWy+tbXnBXp8OfnxoTGpdED/GQeoKJXTPIUj5JH3HiJlEP2PgfdgOETEzIsolHWzoKMET0YLV92JmmNhn4KDTHL2d6TNvk07Bu23gO3jz5Vu/J5pPH8HjRActFPQuO6R4WhRB9Y25Tz1T6pKNhFaMcxl7xKVhOiBgFCRCKHAtgHI0iznKH/bQQuw06VGz9M9J/GhETpMEhTCtLDI47MoTnlKQn0TS7X3PjqE1fiGXsvHbmAI3t/UX/5pc7GabeM9pS8JVMH+rFM9z1uYLH7kxpxi3gFCgwJmpXLikI6AfQs/HtI2fH6WbanllFu4LAdG81TT+7WmHJsBjJq6RIUzMVs47Hsxh6PUE= X-Forefront-Antispam-Report: CIP:255.255.255.255;CTRY:;LANG:en;SCL:1;SRV:;IPV:NLI;SFV:NSPM;H:SN6PR1501MB2064.namprd15.prod.outlook.com;PTR:;CAT:NONE;SFS:(13230022)(4636009)(136003)(39860400002)(366004)(376002)(346002)(396003)(451199015)(53546011)(41300700001)(2616005)(31686004)(54906003)(6506007)(31696002)(83380400001)(6512007)(6666004)(316002)(86362001)(186003)(4326008)(38100700002)(66476007)(66946007)(66556008)(8936002)(478600001)(36756003)(8676002)(110136005)(6486002)(2906002)(5660300002)(45980500001)(43740500002);DIR:OUT;SFP:1102; X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1 X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?QlRmU2lWbnRyTTFuNUJWL0JQZlU4UDloazNEZHZFZldKU29iS0tISXNseUU5?= =?utf-8?B?TnZBeTRLaENlRHphNG41YXhobmJJc1lpL29OdW05QkZMNm80am1HRGFKU0pq?= =?utf-8?B?dVBlcEhRejZwVlo5WG1wNDFLZ3R1NXlOMnpJMEpiZTA1YURQTW9sVWV2L3A3?= =?utf-8?B?NHJOMFdETkdBRTVYUExHMS94RlR0MGZDUVhPVlkyL1JvSkJXdXY0VUlJYlJS?= =?utf-8?B?MHp2Yk51QnJYYzlYeVh4dWJscWZGT3hTaWFtMFlIa0lzVmNKaTNrbW5kS29k?= =?utf-8?B?TWJYTWZCU1FoSmJiSzRFMkZFejFud2dHL011SGd5eUFJbnNJK1p3amFHeTdh?= =?utf-8?B?ZTl0ekhua1cwZXBZOVBrekh0WHB3cnV1Ty9TUTE0OEI1Y0lrY2FqWlRLVXJM?= =?utf-8?B?ZDQ4c0JFcDBuNTFNdlZaNG4vWXFaQ3RPRldiaVNjVXVQYTl2Wjc5N1JBNEJ2?= =?utf-8?B?MkxZTFFJRzErVGJvT0dWRnZwVkdDMFFpOEcyOHBLdks5cEVKb0lPYXFZam1L?= =?utf-8?B?dWljWUpaZEg5TVo1WjdtQ096Q2xWeVpLcFVPellnN1AvVkxMTTFYOGVoVkYv?= =?utf-8?B?aSs2ellCazIvQit3ZUN0eDhQdkpDaTZhbXozR3dpaStTTFdrTmhoY0RYNnJx?= =?utf-8?B?L3hkb3pQZWd5Sk9iSm1mV1VpL1orNG03ZEhGbVRPMkZRV1JsWEI0WVkyTHhM?= =?utf-8?B?dm9KQlNPNFIwV2I4T2pIQytaSGIvc0R5TkxROVh2a2MyZW5hcjhvczNQM1Bq?= =?utf-8?B?ZWxibjJ3N3p5RVVxT2FrbEhMY1lOQzRtNGJqUm9jR1ROck5Dc0tPRGhVNnFt?= =?utf-8?B?MzZLUGlNZEdTOURFd2RQdlFLdmNBd1I3RWd6dGJDSXB3MmlhSUcxZ0F0YURt?= =?utf-8?B?RjRmbFlnOUlBOC9lRnNIN3FIQnZmZEdxcjY1OVNZREJzTGxEK1JxQnp6VTk3?= =?utf-8?B?ZjV2NFpoa3BwZFpkdHNCblhWVWtYLzZ1VEVrUUtqSG9SaTlPbnhPTU16dlRw?= =?utf-8?B?NFV0YkhqNUFQN1lGR29hM3pTek5qYmlkU1ZKcXB3ZHZ0dFYzNkRzbkVuYXJ1?= =?utf-8?B?S2E2ekJkVzZyeGhVdGRjMEZmWVdDNmZ0RDJGeFN2Sk8ySkREN1lRaEtxMjZZ?= =?utf-8?B?YjRRZ0c4ZTM5Y0doZEZ4VVZ6VTZFdGk1d2V4RjFINXlYMDBvdjV3cVNRTDU5?= =?utf-8?B?L2VzTml4Ri93bDhmR0FnR0RHUVhxbzA4RWplS1FPRGFVaG1rNnZYb0VwenZF?= =?utf-8?B?OUlDcVpWblVJeU1XWG1qWHZudnRyZ2ViVTFqMmQ2ZVpMMDBkTVBBd2tzVHMv?= =?utf-8?B?R0g1aXJ6OGhES0dHbUptbDFibDMvQTNXcWFpQm5MY3RldWd0TDBqaGxnN2Rl?= =?utf-8?B?eVIyRURweDdxOHhtRzFjKzJPeWo3OVUyaFI3ZUE5eGdvaHRaOXVqVCt1am1y?= =?utf-8?B?dG1NZEJGWWlaQ1JkUDVNL01ZTlJTQ3diNFhGeGVYcWdSdXFpeTgycmJjcXBW?= =?utf-8?B?S1owbDZ3c1RnajlwSVE4MGFKS3ExY1VDdWk1ZkxJakRtSE85M1Z4aG1sWWFu?= =?utf-8?B?TmFUeXEybkNMMTZOVldEY3A0TWplTlJXemErV0Y4VXlIQXhFTTB1bWgzUGY2?= =?utf-8?B?S3lVWWVhb09CUG5YL0FZbUVyTy9PQWNLRWRLUjh1TFJxZndnZEhGQWYwUm52?= =?utf-8?B?R05Ncm1pL1lKdThzQ2lNRTQ0TjZiT3FwMEZYMTVCZFNzeDJRdnNhMHhmUktx?= =?utf-8?B?WGxMZVVFazg2UFRFYndMTHdMUmE4ZC9oVGFiUndKeTAyQUs0TGxESENPbzdF?= =?utf-8?B?U0ZDaUJPclBQWnBHUmJyVEwxQjIwSXRiempFMUNlbTU4NG4yUjlKRGJ6ZUxF?= =?utf-8?B?b3hqdWxMSzZ2Z2lwSVVReXVMOGFBZng5T0JPODFLcWgzeXBqYmI3UUxSWGls?= =?utf-8?B?OGN6VG92QTcwTHNiTHUzZ3dZdWh6Q0cyUjdXNlh0NFF5VVZCMGtmbUpIWEJk?= =?utf-8?B?SXNpZlFacTRONUVhcVppQWViNXZmU0REWWxsTE1jYVJUbCtCTmVWbDFhSUZw?= =?utf-8?B?UCtNZUtqVVRRNDdEcDlManJDS2wrKzVtVWJNTWp4VmJuMDJKckl6YkNibFEr?= =?utf-8?B?ZTVwSlZoSFFvZVVFSElSRzRjRkgwYzdpMCtHbk9UdEtTVUNjeHR4Z2VQK1pI?= =?utf-8?B?dGc9PQ==?= X-OriginatorOrg: meta.com X-MS-Exchange-CrossTenant-Network-Message-Id: 2a2aeb35-677f-42ed-6972-08dac794150c X-MS-Exchange-CrossTenant-AuthSource: SN6PR1501MB2064.namprd15.prod.outlook.com X-MS-Exchange-CrossTenant-AuthAs: Internal X-MS-Exchange-CrossTenant-OriginalArrivalTime: 16 Nov 2022 05:33:24.9506 (UTC) X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted X-MS-Exchange-CrossTenant-Id: 8ae927fe-1255-47a7-a2af-5f3a069daaa2 X-MS-Exchange-CrossTenant-MailboxType: HOSTED X-MS-Exchange-CrossTenant-UserPrincipalName: KnGm6aSddCkfBK3j/T3jX3T0Ait9veIldLjLo8zsORUtREmP4bZJK05oSByAMa2H X-MS-Exchange-Transport-CrossTenantHeadersStamped: BN8PR15MB2786 X-Proofpoint-ORIG-GUID: dZ4uwn8w7ARsG_HsxNbOCqfX0Tg5dkzl X-Proofpoint-GUID: dZ4uwn8w7ARsG_HsxNbOCqfX0Tg5dkzl X-Proofpoint-Virus-Version: vendor=baseguard engine=ICAP:2.0.219,Aquarius:18.0.895,Hydra:6.0.545,FMLib:17.11.122.1 definitions=2022-11-15_08,2022-11-15_03,2022-06-22_01 Precedence: bulk List-ID: X-Mailing-List: bpf@vger.kernel.org On 11/14/22 10:50 PM, Martin KaFai Lau wrote: > On 11/11/22 8:58 AM, Yonghong Song wrote: >> diff --git a/tools/testing/selftests/bpf/progs/rcu_read_lock.c >> b/tools/testing/selftests/bpf/progs/rcu_read_lock.c >> new file mode 100644 >> index 000000000000..c11b4f8f9a9d >> --- /dev/null >> +++ b/tools/testing/selftests/bpf/progs/rcu_read_lock.c >> @@ -0,0 +1,355 @@ >> +// SPDX-License-Identifier: GPL-2.0 >> +/* Copyright (c) 2022 Meta Platforms, Inc. and affiliates. */ >> + >> +#include "vmlinux.h" >> +#include >> +#include >> +#include "bpf_tracing_net.h" >> +#include "bpf_misc.h" >> + >> +char _license[] SEC("license") = "GPL"; >> + >> +struct { >> +    __uint(type, BPF_MAP_TYPE_CGRP_STORAGE); >> +    __uint(map_flags, BPF_F_NO_PREALLOC); >> +    __type(key, int); >> +    __type(value, long); >> +} map_a SEC(".maps"); >> + >> +struct { >> +    __uint(type, BPF_MAP_TYPE_TASK_STORAGE); >> +    __uint(map_flags, BPF_F_NO_PREALLOC); >> +    __type(key, int); >> +    __type(value, long); >> +} map_b SEC(".maps"); >> + >> +__u32 user_data, key_serial, target_pid = 0; >> +__u64 flags, result = 0; >> + >> +struct bpf_key *bpf_lookup_user_key(__u32 serial, __u64 flags) __ksym; >> +void bpf_key_put(struct bpf_key *key) __ksym; >> +void bpf_rcu_read_lock(void) __ksym; >> +void bpf_rcu_read_unlock(void) __ksym; >> + >> +SEC("?fentry.s/" SYS_PREFIX "sys_getpgid") >> +int cgrp_succ(void *ctx) >> +{ >> +    struct task_struct *task; >> +    struct css_set *cgroups; >> +    struct cgroup *dfl_cgrp; >> +    long init_val = 2; >> +    long *ptr; >> + >> +    task = bpf_get_current_task_btf(); >> +    if (task->pid != target_pid) >> +        return 0; >> + >> +    bpf_rcu_read_lock(); >> +    cgroups = task->cgroups; >> +    dfl_cgrp = cgroups->dfl_cgrp; >> +    bpf_rcu_read_unlock(); > > Outside of the rcu section, "cgroups" could have been gone.  Is it > possible that "dfl_cgrp" could be gone together with "cgroups"? In this particular case, looks like the best is indeed to get a reference for dfl_cgrp before doing bpf_rcu_read_unlock. But to decide whether a particular non-rcu pointer needs reference or not needs kernel internal knowledge for that particular context. So right now, the approach is to take the approach for generic ptr_to_btf_id approach. That is, the non-rcu pointer after rcu pointer tracing is 'trusted'. The user can always increase bpf_rcu_read_lock() region by himself to cover the case if the non-rcu pointer is untrusted. > >> +    ptr = bpf_cgrp_storage_get(&map_a, dfl_cgrp, &init_val, >> +                   BPF_LOCAL_STORAGE_GET_F_CREATE); >> +    if (!ptr) >> +        return 0; >> +    ptr = bpf_cgrp_storage_get(&map_a, dfl_cgrp, 0, 0); >> +    if (!ptr) >> +        return 0; >> +    result = *ptr; >> +    return 0; >> +} >> + > > [ ... ] > >> +SEC("?fentry.s/" SYS_PREFIX "sys_getpgid") >> +int miss_unlock(void *ctx) >> +{ >> +    struct task_struct *task; >> +    struct css_set *cgroups; >> +    struct cgroup *dfl_cgrp; >> + >> +    /* missing bpf_rcu_read_unlock() */ >> +    bpf_rcu_read_lock(); > > >> +    task = bpf_get_current_task_btf(); >> +    bpf_rcu_read_lock(); > > One of the bpf_rcu_read_lock() needs to be removed.  Otherwise, I think > the verifier will error on the nested rcu read lock first instead of > testing the missing unlock case here. Thanks, will do. > >> +    cgroups = task->cgroups; >> +    bpf_rcu_read_unlock(); >> +    dfl_cgrp = cgroups->dfl_cgrp; >> +    (void)bpf_cgrp_storage_get(&map_a, dfl_cgrp, 0, >> +                   BPF_LOCAL_STORAGE_GET_F_CREATE); >> +    return 0; >> +} >