Subject: Re: [PATCH] optee: Suppress false positive kmemleak report in
 optee_handle_rpc()
To:     Sumit Garg <sumit.garg@linaro.org>,
        Jerome Forissier <jerome@forissier.org>
Cc:     "op-tee@lists.trustedfirmware.org" <op-tee@lists.trustedfirmware.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        Jens Wiklander <jens.wiklander@linaro.org>,
        Etienne Carriere <etienne.carriere@linaro.org>
References: <20211206120533.602062-1-xiaolei.wang@windriver.com>
 <CAFA6WYN+0751=feb-O9Drmm5V_Gz-1qsgiHmLsA88=49MoK_dg@mail.gmail.com>
 <PH0PR11MB507734019F54C2BB24D1456F95719@PH0PR11MB5077.namprd11.prod.outlook.com>
 <CAFA6WYMOHUEve8cbZdwzsijer3fRsy=50q67ndsC6U2JD6gK5Q@mail.gmail.com>
 <ede44051-41db-60b4-d5a3-97a789dd52bc@forissier.org>
 <CAFA6WYM1oCs9gE4b5DaRez+jhCXPb_c25ausj0yWdS5tawX0MA@mail.gmail.com>
From:   wangxiaolei <xiaolei.wang@windriver.com>
Message-ID: <2c9f2f34-9dab-4f1b-bc10-48212fb70335@windriver.com>
Date:   Mon, 13 Dec 2021 16:55:45 +0800
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101
 Thunderbird/78.13.0
In-Reply-To: <CAFA6WYM1oCs9gE4b5DaRez+jhCXPb_c25ausj0yWdS5tawX0MA@mail.gmail.com>
Content-Type: text/plain; charset=utf-8; format=flowed
Content-Transfer-Encoding: 7bit
Content-Language: en-US
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?aFpXYmk1OGJwM0loU2NYUlRSQ05CeDVYQWswRzRRalV3aEtyUVN3MEhnQWpB?=
 =?utf-8?B?WmtvNXlCZ3BFRUNjUnpJRFJ2UFIrOE9GUVZycnZiaVdERHhOc2VYWDAvNnk5?=
 =?utf-8?B?TTFxSE5nekhCTlErdTFEaTQxVU9vQ3dCRG9SdGMzeTNwWSszcmxTN2xWOE43?=
 =?utf-8?B?QXZqeTU0UGFhRHcrWUNBUGlGZEZzR1NTdnVpaGI5M3NGMHNPUzdWMDRtd2ls?=
 =?utf-8?B?dzlsamlaNmV3dDcyQUVTMzdnTU41U0Mycm8yTkp2T09Udmpla1l1ejlpc2Vy?=
 =?utf-8?B?ZXVHY1FoSWxIdHltQmxrQWtydUhpb1YzNEZaZUJPUnEvUG5TY1pINW5NRnVL?=
 =?utf-8?B?c05hRVVmeU5ZSGZ6R1VObTdOdDdCbnl6Ym5VYk55Um1IcWY3WkZhSWFLSTFv?=
 =?utf-8?B?V0F2NHZYY0hmNm5NMnk0UVptRGVBVUlaSkpmWEVvdis1WlVyYk1EWkRQekx3?=
 =?utf-8?B?b1ZteUVZa01ObU9sT3JtOHhyRS9JYlNsd1dMSTNwUGtJbEtjYUNOTTJGQitK?=
 =?utf-8?B?RnZpZDJXVCtXNm9HTkFQN2lzMjlYWHhCdC9IaGQ1dGZtOWtPeG95SlRkL0Yw?=
 =?utf-8?B?QytQY3VtM2Vic2txSzhxdnpWNU0rby93dG1FalpvbkFFZGNtYndHNUU1Ull4?=
 =?utf-8?B?aVVWcDhBcy9nZ0JVc21qbW9tMXcrTHV2VzhxQVcxWVQ0elVJcEhaMGdyeWY2?=
 =?utf-8?B?SGF0K0Fjei9oaW44bG55TEMxRWViRVhad0tjZ1ZEUnNkNXpSRC9FMUtncXBx?=
 =?utf-8?B?d1RXcmdUbFA4NUxTWWdkcjhwd2g4a3RLYWVTbmdsdU5VQjFLb1YrTFYzczdo?=
 =?utf-8?B?U2NYd3hvUjMvaThqL203ZysyMWh4eHJCbFc1Tm9GckNjK3NsQWUrQVg2azd2?=
 =?utf-8?B?R0FUNDJDWnFuaDY0dXdEbTV2UHN4VWc3dndRUXlteFhUL0dFMTJQL1pCOG9q?=
 =?utf-8?B?QUwrTHZvWWZDc0JPOTdXaUVtNXFBK2Rici93Z1ZickNNcGFNTGdnRVZST2hB?=
 =?utf-8?B?bmx5K01KT0FUVDZ2cUM4NTE1cUU3WWdRVEM0MWhkSVl6MnZiVWVyUks1YXBV?=
 =?utf-8?B?NTdvNU1LUWlMOGJaV0hmdStteGQzTTFrdFRsS0F5c2pDODFJZ2Nqa3RCSis4?=
 =?utf-8?B?QnBYR3R3d2V4TXFxeHU2dXNUblVWZmNTOTA1RGp1bHNqam5FKzRnbTkzcUFY?=
 =?utf-8?B?UWVqR3lndzFGT2JrazRQUldZalhoa0FZMVFaRFgxeksyUHpTMFhmZFExSk9L?=
 =?utf-8?B?Ymo3ZFBTWGV1eENFZlUyL1drTUFxUmRmNU1aWkdHMDROUmVtbFZ1ZjhzRFhD?=
 =?utf-8?B?aXhZV3l0SVcyK2d6ckZmMWhiL1E4YXc1cHlZWlhoWFhDWXN6Mks3VUkramFT?=
 =?utf-8?B?WWM0TmN1VStJUGtBMEIweFlrZ1B6VTZKN0VGZEIxbzZMVnJTc01JZlVTSjRB?=
 =?utf-8?B?S292ZUZ4YVUxV1BLSHE1WHB4VjdGV1lLOXFvNHc3MFdxd3k3Q1dPVjcwdDlj?=
 =?utf-8?B?cEYyYndDYXltUlhwYXpHbUNKM1NHREhEYnM2c2RGYUJORG5IajVFLzREc0hQ?=
 =?utf-8?B?K3Jwdmg1MVAvL0lZR3lKdkVweWl2c2p1SC92b0tKY2VhV0t0Z2RuZGIvRTNa?=
 =?utf-8?B?VzJZYnNCbDN2U1dVa3ZwUmpsZ0dLVHBleFFCMVg5RzY4OFZSMTJuMk5nZG1o?=
 =?utf-8?B?bmVWb09Gd3RZWFg3ZlhoYUEwWmxneVdiREM3VWxuUS9MSXI1ek1vN3YycnFp?=
 =?utf-8?B?UER5MkpSRUYrMk92ZnU3Y1Y4bVdVRUR5Rk1FUFJBNis5YlZTWnFNOGlkaTdP?=
 =?utf-8?B?cEhpWDVENWVZdGpBcVRFbnR3MEdHdGsrRGgrN2tCZUJSZGplRXJCWTgyRDF4?=
 =?utf-8?B?dXdlVHBOWi9kcE8xbnFQWGhZaktuQjZBbTA0L1JpK0tlR25zRVpYV0taa1F6?=
 =?utf-8?B?K1hqL2tCNkRiSTVlbkloMkpzVjlycXRBbFExT0dHeEx3NStLN292WXhsak9M?=
 =?utf-8?B?S01ma21uUlQyQmxRNEp2bmdvek8vZC9tV3VJdVJCR1NoUjJqMjJOV2Z4TlFa?=
 =?utf-8?B?cTN2VzBmS1pZb0tMQ0ZNQ2p3Y0dKeGE4cHNHMm1OUm1Ec1hDK1JKanU4T2Z3?=
 =?utf-8?B?WFg3SDAzYXZjVFZPZ1VPYXh1VFpOU3pSZGxXU0ZmNU91L05YWW51VEg0R08w?=
 =?utf-8?B?Z0tXQUkwREpMd2tsNFFwdXhrUHova3c2MHFuL1R4SXl1dEFFQ3YyOU1yc1A3?=
 =?utf-8?B?am1jdnZVbFFyQU9rQVNvOEphV2t3PT0=?=
X-OriginatorOrg: windriver.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 7856a6c0-1ebb-4a86-d2ee-08d9be165dcd
X-MS-Exchange-CrossTenant-AuthSource: PH0PR11MB5077.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 13 Dec 2021 08:55:52.3795
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 8ddb2873-a1ad-4a18-ae4e-4644631433be
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: THkmgAp2AOD3ADFR5gBIXr0XtT4b5otUCG7Vg3hpyc6zvRg+LLByZsUQFJPvh6I+49Zgeudf8ye2jEpPFi0gBR2DwIJ3D/JNQjvWxi9ueyQ=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: PH0PR11MB5173
X-Proofpoint-GUID: piBCVRDROsHek8QIctGD7rC4oqIXU8dc
X-Proofpoint-ORIG-GUID: piBCVRDROsHek8QIctGD7rC4oqIXU8dc
X-Proofpoint-Virus-Version: vendor=baseguard
 engine=ICAP:2.0.205,Aquarius:18.0.790,Hydra:6.0.425,FMLib:17.11.62.513
 definitions=2021-12-13_03,2021-12-10_01,2021-12-02_01
X-Proofpoint-Spam-Details: rule=outbound_notspam policy=outbound score=0 suspectscore=0
 mlxlogscore=999 clxscore=1011 malwarescore=0 lowpriorityscore=0
 impostorscore=0 bulkscore=0 adultscore=0 priorityscore=1501 mlxscore=0
 phishscore=0 spamscore=0 classifier=spam adjust=0 reason=mlx scancount=1
 engine=8.12.0-2110150000 definitions=main-2112130057
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org


On 12/10/21 5:38 PM, Sumit Garg wrote:
> [Please note: This e-mail is from an EXTERNAL e-mail address]
>
> On Fri, 10 Dec 2021 at 13:40, Jerome Forissier <jerome@forissier.org> wrote:
>> +CC Jens, Etienne
>>
>> On 12/10/21 06:00, Sumit Garg wrote:
>>> On Fri, 10 Dec 2021 at 09:42, Wang, Xiaolei <Xiaolei.Wang@windriver.com> wrote:
>>>> -----Original Message-----
>>>> From: Sumit Garg <sumit.garg@linaro.org>
>>>> Sent: Thursday, December 9, 2021 7:41 PM
>>>> To: Wang, Xiaolei <Xiaolei.Wang@windriver.com>
>>>> Cc: jens.wiklander@linaro.org; op-tee@lists.trustedfirmware.org; linux-kernel@vger.kernel.org
>>>> Subject: Re: [PATCH] optee: Suppress false positive kmemleak report in optee_handle_rpc()
>>>>
>>>> [Please note: This e-mail is from an EXTERNAL e-mail address]
>>>>
>>>> On Mon, 6 Dec 2021 at 17:35, Xiaolei Wang <xiaolei.wang@windriver.com> wrote:
>>>>> We observed the following kmemleak report:
>>>>> unreferenced object 0xffff000007904500 (size 128):
>>>>>    comm "swapper/0", pid 1, jiffies 4294892671 (age 44.036s)
>>>>>    hex dump (first 32 bytes):
>>>>>      00 47 90 07 00 00 ff ff 60 00 c0 ff 00 00 00 00  .G......`.......
>>>>>      60 00 80 13 00 80 ff ff a0 00 00 00 00 00 00 00  `...............
>>>>>    backtrace:
>>>>>      [<000000004c12b1c7>] kmem_cache_alloc+0x1ac/0x2f4
>>>>>      [<000000005d23eb4f>] tee_shm_alloc+0x78/0x230
>>>>>      [<00000000794dd22c>] optee_handle_rpc+0x60/0x6f0
>>>>>      [<00000000d9f7c52d>] optee_do_call_with_arg+0x17c/0x1dc
>>>>>      [<00000000c35884da>] optee_open_session+0x128/0x1ec
>>>>>      [<000000001748f2ff>] tee_client_open_session+0x28/0x40
>>>>>      [<00000000aecb5389>] optee_enumerate_devices+0x84/0x2a0
>>>>>      [<000000003df18bf1>] optee_probe+0x674/0x6cc
>>>>>      [<000000003a4a534a>] platform_drv_probe+0x54/0xb0
>>>>>      [<000000000c51ce7d>] really_probe+0xe4/0x4d0
>>>>>      [<000000002f04c865>] driver_probe_device+0x58/0xc0
>>>>>      [<00000000b485397d>] device_driver_attach+0xc0/0xd0
>>>>>      [<00000000c835f0df>] __driver_attach+0x84/0x124
>>>>>      [<000000008e5a429c>] bus_for_each_dev+0x70/0xc0
>>>>>      [<000000001735e8a8>] driver_attach+0x24/0x30
>>>>>      [<000000006d94b04f>] bus_add_driver+0x104/0x1ec
>>>>>
>>>>> This is not a memory leak because we pass the share memory pointer to
>>>>> secure world and would get it from secure world before releasing it.
>>>>> How about if it's actually a memory leak caused by the secure world?
>>>>> An example being secure world just allocates kernel memory via OPTEE_SMC_RPC_FUNC_ALLOC and doesn't free it via OPTEE_SMC_RPC_FUNC_FREE.
>>>>> IMO, we need to cross-check optee-os if it's responsible for leaking kernel memory.
>>>> Hi sumit,
>>>>
>>>> You mean we need to check whether there is a real memleak,
>>>> If being secure world just allocate kernel memory via OPTEE_SMC_PRC_FUNC_ALLOC and until the end, there is no free
>>>> It via OPTEE_SMC_PRC_FUNC_FREE, then we should judge it as a memory leak, wo need to judge whether it is caused by secure os?
>>> Yes. AFAICT, optee-os should allocate shared memory to communicate
>>> with tee-supplicant. So once the communication is done, the underlying
>>> shared memory should be freed. I can't think of any scenario where
>>> optee-os should keep hold-off shared memory indefinitely.
>> I believe it can happen when OP-TEE's CFG_PREALLOC_RPC_CACHE is y. See
>> the config file [1] and the commit which introduced this config [2].
> Okay, I see the reasoning. So during the OP-TEE driver's lifetime, the
> RPC shared memory remains allocated. I guess that is done primarily
> for performance reasons.
>
> But still it doesn't feel appropriate that we term all RPC shm
> allocations as not leaking memory as we might miss obvious ones.
>
> Xiaolei,
>
> Can you once test with CFG_PREALLOC_RPC_CACHE=n while compiling
> optee-os and see if the observed memory leak disappears or not?
>
> -Sumit

Hi sumit


The version I am using has not increased the CFG_PREALLOC_RPC_CACHE

switch, I checked out to the latest version, but because of the need for

additional patches for the imx8 platform, I still have no way to test the

CFG_PREALLOC_RPC_CACHE=n situation


thanks

xiaolei

>
>> [1] https://github.com/OP-TEE/optee_os/blob/3.15.0/mk/config.mk#L709
>> [2] https://github.com/OP-TEE/optee_os/commit/8887663248ad
>>
>> --
>> Jerome