Received-SPF: pass (google.com: domain of linux-kernel+bounces-60054-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.48.161 as permitted sender) client-ip=147.75.48.161;
Message-ID: <4237a933-0f61-417f-bbb6-ce5954b304d4@intel.com>
Date: Sat, 10 Feb 2024 05:53:25 +0800
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH] dmaengine: idxd: Ensure safe user copy of completion
 record
Content-Language: en-US
To: Fenghua Yu <fenghua.yu@intel.com>, Vinod Koul <vkoul@kernel.org>, "Dave
 Jiang" <dave.jiang@intel.com>
CC: <dmaengine@vger.kernel.org>, linux-kernel <linux-kernel@vger.kernel.org>,
	Tony Zhu <tony.zhu@intel.com>
References: <20240209191412.1050270-1-fenghua.yu@intel.com>
From: Lijun Pan <lijun.pan@intel.com>
In-Reply-To: <20240209191412.1050270-1-fenghua.yu@intel.com>
Content-Type: text/plain; charset="UTF-8"; format=flowed
Content-Transfer-Encoding: 7bit
Precedence: bulk
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?T3FsUXNvTkU1K1hEamVQOCtZTU9Ec3pSZjAyNmM2RzFvaHgwTzhSK0dqeDla?=
 =?utf-8?B?c0VuU2wwN1dqdjZ5d21Xb3QwQ3BUbFkrbEtmZ0h1RnJ4RDRWZldHNDNuMHBE?=
 =?utf-8?B?QVFqWkFOZWdtWklJZndoejROUVcyajA2RjB6bjRaM3A1UUZFOXlGS0JQM1I4?=
 =?utf-8?B?SUdsK05yM2ZVYWFyUFZ4RGc5YXJmK3NHMXhtNzZGM2J1dm5WWWxNT3hScVg0?=
 =?utf-8?B?alRqaWc2NmVpdzh6YW5sR3lXYldKcWwvZTFrbXNDWHppQm5ZQkZhSEg0Zmcx?=
 =?utf-8?B?UldHTjg3c3YwOXVsVVIxTG1OcWMyRFFFYkZOaDd0L1MrWk1FTEJLbkVsakEx?=
 =?utf-8?B?QkQ3QlBmNEVMWjlkTjBRYW5QekhWZWVrb1EwU0pwSlk5M1Z3UFh2YmErUHdH?=
 =?utf-8?B?VEVaWGlWalAxdmRlVnd3bmM4Uk5yR0kzQkFuZnUzcGo5QUhDSGZLVnpjR0lP?=
 =?utf-8?B?NjBjNUpWODlhZ011Vy95a2ZuMi9pWlBYZkQySVRxZ1hkL29hK0RjWlVuNi9Y?=
 =?utf-8?B?YjRtN2VTTU9LaFJaZXNaaWNZRkNQcHFpRWdNQ2I2VGljMHpCd0I1QzRrR0N0?=
 =?utf-8?B?WVdLK291QlRUMk1iazNnZ1ByT1FvRVY5aWR3MTRMMmVpcDJUWXltaFBWcnI4?=
 =?utf-8?B?L0tTclpBTFpGMnlodDluWjlkOHlqMElzRXlkZ1R1ZFhKMVQvQkVOTEU0eWNI?=
 =?utf-8?B?ZWt4NDRoTEcxKzBHUW1hUm9sZUtpTHB4Sy9YVW13Tm1QcFJ4VFF4bWh3WDV5?=
 =?utf-8?B?RUpCSFBXOGRoQ09IbUM5RWY0WWdCNmZsV2wrdjNpR3dFa0Y5UVZFZG1yMHI1?=
 =?utf-8?B?MVlTL0VpOVdueGF5b1VVTlBLWExpQlR5c3hsWGZKSTViTm9CTlBjdWZ2UEFv?=
 =?utf-8?B?VDZGcFMyTm9GNnBHZTl0VTZHWDJLcUtZLytNcENiMnhQU21OZTVsYzVPOXY1?=
 =?utf-8?B?K1BHQkRSVHJqTXF0U0RZdmJKTWNuR3AvU1U3RDdCR0RGeUJNNVdsa2xzempV?=
 =?utf-8?B?aHFodnhqZVAvQXFzMk5xb1o3WUU3dlovd0ZXUm9QLzFZbjdzbEg2UnFRdTJQ?=
 =?utf-8?B?RTJVbmdic0NLNUpKeWtRUDNTS1BZOXIzc1JlRUIwRmZOaG0xT3pHdmVrVTNv?=
 =?utf-8?B?RXB0YzIyMndvMVpITGZZRUFreWdCU3dsbm5KMU0rTnNSU0NDU1k5MSs1YzM2?=
 =?utf-8?B?MERKa2dkUkJ5RkxaRDhQenAvOXppb0M0L3N3elRLOEEvT2FKVHdDSXJDaEdk?=
 =?utf-8?B?c1dicGZCTXYwSTBEUng3RlJyUHVaUW1uSDJxclRyMjdPeVIwUU4wN2xLdVJm?=
 =?utf-8?B?MW1PQm5xd3ZnWTU2bFRWakl6cEMrVzFIb0V1TjR2ZGJoMFZRUVZUaGpuRU5x?=
 =?utf-8?B?Z21hZlRhZDZrMFBkTVpRU0ZGTEFBTXR5V1kyc0FsSlhhbTI2REc3Zy9LbW9a?=
 =?utf-8?B?Y21oaUI4Y3ZVWXRJQkVHblVVbmpyTHZBTUF5WSszUU00aHhoWVl1cTkydzY5?=
 =?utf-8?B?MjNBYkZzeTNzZC9tQk8yWVJjZHhnc3V0Y25YeHNTdllPbHRzcjR5dUI3bmtX?=
 =?utf-8?B?VDNocVlkNWg0R1JnU1ovMnJtbmNGaHpQemFvYk9VODZ2N2RBZi9KSTEvQndp?=
 =?utf-8?B?SWhUVGtNb3pkVW0wVnZJV0k4QnZTK3RqbHh0ZWc3R1g2WnlPNE9KQThscmNB?=
 =?utf-8?B?UFY4TVo3ZnVWOHl1UWZOS3ByZisvbDVkcXh5TkxiZFhncm0rV29MVmFuREJj?=
 =?utf-8?B?VDFyU3ljbFp5eXI2UVZyTmN0MXBhNThZeE5vMFRyZ3NueHVKZm4yeTF5ZkhK?=
 =?utf-8?B?cEcrL1cxR290RUlmc2hwcm1oNk5yb1RXVFlueDRjNlNIcGlreWpFVTlaVC8y?=
 =?utf-8?B?YVovVFk5NXpvc1l1WWFQb3pjUnlIM2x0bU9ZRFF3SFJTWW84RzBLNFoxRjhW?=
 =?utf-8?B?S21icTFGd3RwVEdsVllnUW5kUHNvSEV6TUEzZTVQL2FjVEM0M0NRSHN5WldM?=
 =?utf-8?B?dzlUSkhDREJJMjNNZ2htWVVhSXZ6amkwZGRyb0JlN3RvaHJld2dIT1NSZ0hE?=
 =?utf-8?B?V0xVWjkxSzROajlMM0tod0xPWnU5RW5kZk1jRlR1cDlyNEFSUkh5L3VONTRh?=
 =?utf-8?Q?xZV2aQqYrrn7KPlXGTJLjLAXD?=
X-MS-Exchange-CrossTenant-Network-Message-Id: 0f53155d-057e-4d93-6cf2-08dc29b98fee
X-MS-Exchange-CrossTenant-AuthSource: PH8PR11MB7141.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 09 Feb 2024 21:53:34.3611
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 46c98d88-e344-4ed4-8496-4ed7712e255d
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: cIsu+S53HsYRtPYP0NOmD878PP2r/rViiAUUNVtrU12edu9LM05jGbdR3CbKIV96/Aog46+HOZHqRG4IaBHFrg==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: PH7PR11MB6796
X-OriginatorOrg: intel.com


On 2/10/2024 3:14 AM, Fenghua Yu wrote:
> If CONFIG_HARDENED_USERCOPY is enabled, copying completion record from
> event log cache to user triggers a kernel bug.
> 
> [ 1987.159822] usercopy: Kernel memory exposure attempt detected from SLUB object 'dsa0' (offset 74, size 31)!
> [ 1987.170845] ------------[ cut here ]------------
> [ 1987.176086] kernel BUG at mm/usercopy.c:102!
> [ 1987.180946] invalid opcode: 0000 [#1] PREEMPT SMP NOPTI
> [ 1987.186866] CPU: 17 PID: 528 Comm: kworker/17:1 Not tainted 6.8.0-rc2+ #5
> [ 1987.194537] Hardware name: Intel Corporation AvenueCity/AvenueCity, BIOS BHSDCRB1.86B.2492.D03.2307181620 07/18/2023
> [ 1987.206405] Workqueue: wq0.0 idxd_evl_fault_work [idxd]
> [ 1987.212338] RIP: 0010:usercopy_abort+0x72/0x90
> [ 1987.217381] Code: 58 65 9c 50 48 c7 c2 17 85 61 9c 57 48 c7 c7 98 fd 6b 9c 48 0f 44 d6 48 c7 c6 b3 08 62 9c 4c 89 d1 49 0f 44 f3 e8 1e 2e d5 ff <0f> 0b 49 c7 c1 9e 42 61 9c 4c 89 cf 4d 89 c8 eb a9 66 66 2e 0f 1f
> [ 1987.238505] RSP: 0018:ff62f5cf20607d60 EFLAGS: 00010246
> [ 1987.244423] RAX: 000000000000005f RBX: 000000000000001f RCX: 0000000000000000
> [ 1987.252480] RDX: 0000000000000000 RSI: ffffffff9c61429e RDI: 00000000ffffffff
> [ 1987.260538] RBP: ff62f5cf20607d78 R08: ff2a6a89ef3fffe8 R09: 00000000fffeffff
> [ 1987.268595] R10: ff2a6a89eed00000 R11: 0000000000000003 R12: ff2a66934849c89a
> [ 1987.276652] R13: 0000000000000001 R14: ff2a66934849c8b9 R15: ff2a66934849c899
> [ 1987.284710] FS:  0000000000000000(0000) GS:ff2a66b22fe40000(0000) knlGS:0000000000000000
> [ 1987.293850] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> [ 1987.300355] CR2: 00007fe291a37000 CR3: 000000010fbd4005 CR4: 0000000000f71ef0
> [ 1987.308413] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
> [ 1987.316470] DR3: 0000000000000000 DR6: 00000000fffe07f0 DR7: 0000000000000400
> [ 1987.324527] PKRU: 55555554
> [ 1987.327622] Call Trace:
> [ 1987.330424]  <TASK>
> [ 1987.332826]  ? show_regs+0x6e/0x80
> [ 1987.336703]  ? die+0x3c/0xa0
> [ 1987.339988]  ? do_trap+0xd4/0xf0
> [ 1987.343662]  ? do_error_trap+0x75/0xa0
> [ 1987.347922]  ? usercopy_abort+0x72/0x90
> [ 1987.352277]  ? exc_invalid_op+0x57/0x80
> [ 1987.356634]  ? usercopy_abort+0x72/0x90
> [ 1987.360988]  ? asm_exc_invalid_op+0x1f/0x30
> [ 1987.365734]  ? usercopy_abort+0x72/0x90
> [ 1987.370088]  __check_heap_object+0xb7/0xd0
> [ 1987.374739]  __check_object_size+0x175/0x2d0
> [ 1987.379588]  idxd_copy_cr+0xa9/0x130 [idxd]
> [ 1987.384341]  idxd_evl_fault_work+0x127/0x390 [idxd]
> [ 1987.389878]  process_one_work+0x13e/0x300
> [ 1987.394435]  ? __pfx_worker_thread+0x10/0x10
> [ 1987.399284]  worker_thread+0x2f7/0x420
> [ 1987.403544]  ? _raw_spin_unlock_irqrestore+0x2b/0x50
> [ 1987.409171]  ? __pfx_worker_thread+0x10/0x10
> [ 1987.414019]  kthread+0x107/0x140
> [ 1987.417693]  ? __pfx_kthread+0x10/0x10
> [ 1987.421954]  ret_from_fork+0x3d/0x60
> [ 1987.426019]  ? __pfx_kthread+0x10/0x10
> [ 1987.430281]  ret_from_fork_asm+0x1b/0x30
> [ 1987.434744]  </TASK>
> 
> The issue arises because event log cache is created using
> kmem_cache_create() which is not suitable for user copy.
> 
> Fix the issue by creating event log cache with
> kmem_cache_create_usercopy(), ensuring safe user copy.
s/, ensuring/ to ensure


> 
> Fixes: c2f156bf168f ("dmaengine: idxd: create kmem cache for event log fault items")
> Reported-by: Tony Zhu <tony.zhu@intel.com>
> Tested-by: Tony Zhu <tony.zhu@intel.com>
> Signed-off-by: Fenghua Yu <fenghua.yu@intel.com>

Reviewed-by: Lijun Pan <lijun.pan@intel.com>

> ---
>   drivers/dma/idxd/init.c | 15 ++++++++++++---
>   1 file changed, 12 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/dma/idxd/init.c b/drivers/dma/idxd/init.c
> index 14df1f1347a8..4954adc6bb60 100644
> --- a/drivers/dma/idxd/init.c
> +++ b/drivers/dma/idxd/init.c
> @@ -343,7 +343,9 @@ static void idxd_cleanup_internals(struct idxd_device *idxd)
>   static int idxd_init_evl(struct idxd_device *idxd)
>   {
>   	struct device *dev = &idxd->pdev->dev;
> +	unsigned int evl_cache_size;
>   	struct idxd_evl *evl;
> +	const char *idxd_name;
>   
>   	if (idxd->hw.gen_cap.evl_support == 0)
>   		return 0;
> @@ -355,9 +357,16 @@ static int idxd_init_evl(struct idxd_device *idxd)
>   	spin_lock_init(&evl->lock);
>   	evl->size = IDXD_EVL_SIZE_MIN;
>   
> -	idxd->evl_cache = kmem_cache_create(dev_name(idxd_confdev(idxd)),
> -					    sizeof(struct idxd_evl_fault) + evl_ent_size(idxd),
> -					    0, 0, NULL);
> +	idxd_name = dev_name(idxd_confdev(idxd));
> +	evl_cache_size = sizeof(struct idxd_evl_fault) + evl_ent_size(idxd);
> +	/*
> +	 * Since completion record in evl_cache will be copied to user
> +	 * when handling completion record page fault, need to create
> +	 * the cache suitable for user copy.
> +	 */

Maybe briefly compare kmem_cache_create() with 
kmem_cache_create_usercopy() and add up to the above comments. If you 
think it too verbose, then forget about it.

> +	idxd->evl_cache = kmem_cache_create_usercopy(idxd_name, evl_cache_size,
> +						     0, 0, 0, evl_cache_size,
> +						     NULL);
>   	if (!idxd->evl_cache) {
>   		kfree(evl);
>   		return -ENOMEM;