From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on gnuweeb.org X-Spam-Level: X-Spam-Status: No, score=-1.2 required=5.0 tests=DKIM_SIGNED,DKIM_VALID, DKIM_VALID_AU,DKIM_VALID_EF,FREEMAIL_FROM,NICE_REPLY_A, RCVD_IN_DNSWL_NONE,SPF_HELO_NONE,SPF_PASS autolearn=ham autolearn_force=no version=3.4.6 Received: from sonic301-46.consmr.mail.sg3.yahoo.com (sonic301-46.consmr.mail.sg3.yahoo.com [106.10.242.109]) by gnuweeb.org (Postfix) with ESMTPS id 533E780B33 for ; Mon, 29 Aug 2022 04:56:11 +0000 (UTC) Authentication-Results: gnuweeb.org; dkim=pass (2048-bit key; unprotected) header.d=yahoo.com header.i=@yahoo.com header.a=rsa-sha256 header.s=s2048 header.b=SkXeroBR; dkim-atps=neutral DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=yahoo.com; s=s2048; t=1661748968; bh=FOQT6MCx4SwR1DZdxrL6d4eqBis58y7t0uLjZewvST0=; h=Date:To:Cc:References:From:Subject:In-Reply-To:From:Subject:Reply-To; b=SkXeroBRMD8Cy/bGIoBxZTJjNqN5SMW4HS1f6Ol9MgMqGJU/4ExWBZTd1N0+c/M5J0I3TnCGm8mXPeg+mWOK0AA7AJdAae11QY5A8KTEaXB+K3sFXWOOMbpKmlIXggDMv0tfyOmUsch7/CqSqHmY2q5dyeINkFSXemlP+ISCEDK6JNjlVV9x0Bri3FzKwIfmB+I2MZl28lQ+my8kgqrbWl/m0VxrsuKtD1Iepegq8Fz8kY7sy6NvKQEHMxsP2fmE1EJ77Gndt2eQijy/vi2nuuptuIYR9iukFFgyyuPt0IdlcW1RM29onqgE26i7yGfRqFgZISwHimswgSjQ+XoDHA== X-SONIC-DKIM-SIGN: v=1; a=rsa-sha256; c=relaxed/relaxed; d=yahoo.com; s=s2048; t=1661748968; bh=Zc1ik4Jvws49FGLKV227JgoPRIhydbhB+bS+Fi71lko=; h=X-Sonic-MF:Date:To:From:Subject:From:Subject; b=I5EpbGJQ6zE3mKdWJt4tDm5e9zjr7BbcDpB8wR+506Jpg4cHwn6WMVYdliV5V1i3utwqAmuPaUjRFaEYjlVKDHhdyi/1cxppStJY9VC1cJRHLdecB/Bbk9Ft3HNrCuGmpSLNHROHlMMMC26RFXdwTo0GFw/Tgwgeu4czqobSqqZrUdFshGsQoU62r2+0HqobKnxvanbhClzYh5QXOO5WU4QDcAZm/z8mzljh5sDf2zhP7G1ykUotWEXJvWoP8i+b6vidZn6j6WXbJ923xqk4yHDgZDpooJ00ZqyT3T4eAEiZg1n3ZbBuoBeCozBX+ghZbcMGcWkN0MZ+Dv2gvQGmHA== X-YMail-OSG: .HVzxIMVM1n_CCAeBBE0F2mhAdJoJruUf9aqnq73YbsMCeqNn4UJ0WBL4NHiw1K Z3OqClS_ET_5pb6kXtYaUKlIeiRFol29C4FAxQa20uMY3bQHqcjSaqjoYRiTEGeZ7yOGQno9S4Hk phvGV9c1q8ejSDlaORwt84lbkP5dtAH_474MtlTWlOO.cpR9lesOYEtBbaDQv.cnO9uiVJsicBMB ei.41Az1lKAqsfDaQogyReqcLJShCxzFTvEbIz18PYKPtQGa8fANTqYsT7zN1nGtqDimTFUmO1Td eSLMnI1cwEYCtk822zijTRVUw_Byiu1JAaXvPBQJ6EE39IlskRYJ5zJpaGO7jbdNE6gG1cA8jIue Nx2GN7L57Y0G17w3KwtVXmO1gXEtByfxlO3tm9zuuUtP4yoA91C8WOYrXnxZbeKCSqsV7mNZVh4J fQIVNUMzOCukYnUi4ZMU0byldVeQBZO8_oxSGHRxbB.oFNmVrZCLhi_P4z7QE9wbawN_nAHy69fQ asbfMH8AmDJ2OL8yJCm6VUsWHNoJwjeo0SdGxdK6Y3gaEVN8mLV6gsf5_WL8P6fKI7tGJKoa94w1 SIM2vfeJzBEGdLGrY7glAWqWazMoT_xwH8GSOCoZnIpYwRrh61vVX6EH06pmRSHY_QvCaHn47wKA Cd.oMf5DbVypGpAhdrB.FwUVu4R83.j.0s0rKE3aWuGxLTuFmWBUL_yIp32.nEVkuuObqLzFK1F1 ahqjVX92jRD9NFCzji01Kf_7Ey3XLQEUHM4oj_b4bAJdGzYRDfinP0MdnRsTI5R.Z29FI7ye9pRM Xd8HUdQYUGRmaIZdTB5KRYMolF42TJcxAvzPS70kGBKZ392wEWfBrTzh6yU1oNDYe8fTbeqlZAYS i0g9QHZFrNIXEBJhVbq3LmwMCOP_dSZAXtODKpCTmJYYxFeMlwJR6oLDKCHcFpRIkfekmSvEG8ye 9drVyvolXlE2MPLFYXY4w2DeCdpGKYykeNpUNCgJEEqmi.FD9msL2sB2N8tWmhqE.Z3b_7UAmUy. EPkoIfDcymOoC4VyohR8I6V1NpQ4TuFkWibWLqvyW9xXjegH17yZYd76vF6Ey0ST1P3yhovU2zhN MNMVuCTidxNda93O0meQSec36NS7jtMYOW58x_rwBslOhTcb8eCwuXqv3jDTLaG6MxeSkg7T08qz 6oKgIUt_tOEZMWPjNDHNg2tD5aJtzkC618lfiQTaR1avVme9A6hhcSbR_4mWeDJnNqbEuxfVwl8L SN2Q3iPRZWc39YMKbcubBUSA30G8RTm6t8tLde9jcq0iBKZtGdFMNIs_PYqqcSoQ_58UqeL_8pKV RTWQHi_gZdRcPnlkXq7HlJmS1qaURczHNcperPIds08EAm6J1FSaXWg5IkQSKmkIG49e3PxuJfyh 7uzZRIQXFWVD.USn2aNOSaqbVYVcMZKrHx4cY2SPu5bWgYkVN1XvlqsX574BiC01UQhK4iASF_7F Nhn_fXOxY5JWqq.SzJCrd6c_v.Atn3HEfXbTaNtM6kcDwWQr12NKmSmtSoQJtC4dj7h2hMIOv6Dv L4xhlJx2JIunlN0wiKbbGq41SBGoyOFmUFC1AtLSepUUlkQvviuPA5.DsW6utqnUQhdxW3RvFHJV Wf.asrN.oKsA_cqip.1lHrw2DttlVXaW5Z9UIwicjNNnFUMkMKSTiQEUXD0RwpdO2.sxUFH7kwuy QN5d7ib4wYfYem64axgxymL6KO4JW7w10EYGR3FMcBMUZOrQp8Y.Qzz9cGldnbA4RyLFzSn08ov0 rCiQnYz1PWx25HwddQkMDZ.jYIs4i5XdUUybTeTQ7djoS.MgqAjoeG6yrHZoxF0TC.oEhIOAcEkp CuZseTP.tbnApRYdQdOev22Tbd6S4Z7wEbWJsSWCvv_gRkVyJUb78vUFDOLZavCki3ArGmn7m4Kv Nfl2ZxLeuqtTHZKl8wxQugiuQOuNjPk6OnIGbQoeiLCXIPO6NzOlFztqxfHzn4HlayfDlY3oPJC_ V9Kq.zzst2mFL0cOj3I7TPO2_THcfBObORW.8bBNYX4ObX0ImBkYgAhfMsqpbf6py3747ROaRK9C 3FzQvaBwckQiIXxJ6SKFEmokr8uiCDEMbfa1tSC0EdNqEbnZUA5uKP3QtkyWjVXWXMjctg_sSLVc fTSsIDISPD1khQbe0eSs6t8tEmhZansvmIT3tn82k_jXMSUwCEvj39_cKe.KB_B3t7Q7vbJCsX7. 1F9sZlz1HIidjZnNmYFUqkKTgs6243bjWHM0NCh9ZxgRYf_eFkmua_mT1665CW2Ee4HFZ_M.e6OC tWFt8xczt X-Sonic-MF: Received: from sonic.gate.mail.ne1.yahoo.com by sonic301.consmr.mail.sg3.yahoo.com with HTTP; Mon, 29 Aug 2022 04:56:08 +0000 Received: by hermes--canary-production-sg3-6f58cd9b5-84qt6 (Yahoo Inc. Hermes SMTP Server) with ESMTPA ID 635085f2d13d80b92a1c3bff30e6621e; Mon, 29 Aug 2022 04:54:06 +0000 (UTC) Message-ID: Date: Mon, 29 Aug 2022 11:54:03 +0700 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101 Thunderbird/91.11.0 Content-Language: en-US To: Alviro Iskandar Setiawan Cc: Ammar Faizi , Muhammad Rizki , Kanna Scarlet , GNU/Weeb Mailing List References: <20220829011127.3150320-1-ammarfaizi2@gnuweeb.org> <20220829011127.3150320-3-ammarfaizi2@gnuweeb.org> From: Ammar Nofan Faizi Subject: Re: [RFC PATCH v1 2/2] chnet: Implement `get_thread()` and `put_thread()` function In-Reply-To: Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit X-Mailer: WebService/1.1.20595 mail.backend.jedi.jws.acl:role.jedi.acl.token.atz.jws.hermes.yahoo List-Id: On 8/29/22 11:41 AM, Alviro Iskandar Setiawan wrote: > On Mon, Aug 29, 2022 at 8:11 AM Ammar Faizi wrote: >> @@ -251,7 +254,7 @@ net::DefineNetworkTrafficAnnotation("CHNetDelegate", R"( >> })"); >> >> CHNetDelegate::CHNetDelegate(void): >> - thread_("chromium_thread"), >> + thread_(*get_thread()), >> method_("GET"), >> err_("") >> { >> @@ -287,6 +290,7 @@ CHNetDelegate::~CHNetDelegate(void) >> r->PostTask(FROM_HERE, base::BindOnce(CHNetDelegateDestruct, &url_req_, >> &url_req_ctx_, &sig)); >> sig.Wait(); >> + put_thread(&thread_); >> } > > if @url_req_ and @url_req_ctx_ are both nullptr, this put_thread() > won't be called and we have a ref count leak Yes, you're right. Will fix it in the v2 revision. >> template >> @@ -629,6 +633,81 @@ static uint32_t g_max_ch_thpool; >> static std::mutex g_thpool_lock_; >> static struct ch_thpool **g_thpool; >> >> + >> +static base::Thread *get_thread(void) >> +{ >> + const uint32_t max_ch_thpool = g_max_ch_thpool; >> + const uint32_t nr_ref_split = 2048; >> + struct ch_thpool **thp; >> + struct ch_thpool *ret = nullptr; >> + struct ch_thpool *tmp; >> + uint32_t min_ref_idx; >> + uint32_t min_ref; >> + uint32_t i; >> + >> + g_thpool_lock_.lock(); >> + thp = g_thpool; >> + if (!thp) { >> + g_thpool_lock_.unlock(); >> + return nullptr; >> + } > > in what situation @thp can be nullptr? When the chnet_global_destroy() is called. >> + tmp = thp[0]; >> + if (!tmp) { >> + ret = new struct ch_thpool; >> + ret->idx_ = 0; >> + thp[0] = ret; >> + goto out; >> + } >> + >> + min_ref = tmp->ref_count_; >> + min_ref_idx = 0; >> + for (i = 1; i < max_ch_thpool; i++) { >> + uint32_t ref; >> + >> + tmp = thp[i]; >> + if (!tmp) { >> + ret = new struct ch_thpool; >> + ret->idx_ = i; >> + thp[i] = ret; >> + goto out; >> + } >> + >> + ref = tmp->ref_count_; >> + if (ref < nr_ref_split) { >> + ret = tmp; >> + break; >> + } >> + >> + if (ref < min_ref) { >> + min_ref = ref; >> + min_ref_idx = i; >> + } >> + } >> + >> + if (!ret) >> + ret = thp[min_ref_idx]; >> + >> +out: >> + ret->ref_count_++; >> + g_thpool_lock_.unlock(); >> + return &ret->thread_; >> +} > > this unlock() call will behave as a full memory barrier for that > @ref_count_ increment, is this really needed? you can have the > increment after unlock() tho No, the ref_count needs to be protected by a mutex. Otherwise, we have a use-after-free bug. Possible UAF scenario: Thread1 Thread2 ---- ------- --> get_thread() lock() unlock() --> put_thread() # preempted away lock() decrement delete unlock() increment # UAF!!! return return -- Ammar Faizi