Re: [PATCH] crypto: ccp - zero the cmd data after use it
On 8/4/20 12:22, Herbert Xu wrote: > On Tue, Aug 04, 2020 at 12:20:21PM +0800, Liwei Song wrote: >> >> Yes, the other process should do this zero work, but the case I met is >> this address will appear in the slab_alloc_node() as freelist pointer of >> slub, >> and before slub do zero wrok, even kzalloc() doesn't work with this address. > > That would be memory corruption which has nothing to do with your > patch. If it is occurring then you should fix the place that is > corrupting the memory and not work around it like this. OK, understand, thanks for your suggestion. Liwei. > > Cheers, >
Re: [PATCH] crypto: ccp - zero the cmd data after use it
On Tue, Aug 04, 2020 at 12:20:21PM +0800, Liwei Song wrote: > > Yes, the other process should do this zero work, but the case I met is > this address will appear in the slab_alloc_node() as freelist pointer of slub, > and before slub do zero wrok, even kzalloc() doesn't work with this address. That would be memory corruption which has nothing to do with your patch. If it is occurring then you should fix the place that is corrupting the memory and not work around it like this. Cheers, -- Email: Herbert Xu Home Page: http://gondor.apana.org.au/~herbert/ PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
Re: [PATCH] crypto: ccp - zero the cmd data after use it
On 8/4/20 12:04, Herbert Xu wrote: > On Tue, Aug 04, 2020 at 11:51:47AM +0800, Liwei Song wrote: >> >> On 8/3/20 20:52, Herbert Xu wrote: >>> On Mon, Aug 03, 2020 at 03:58:58PM +0800, Liwei Song wrote: exist the following assignment in ccp(ignore the force convert of the struct) by list_del in ccp_dequeue_cmd(): req->__ctx->cmd->entry->next = LIST_POISON1; after use the req, kzfree(req) can not zero the entry entry->next = LIST_POISON1 of the ccp_cmd(cmd) struct when this address available as slub freelist pointer, this will cause the following "general protection fault" error if some process meet this LIST_POISON1 value address when request memory: >>> >>> Your description makes no sense. Please rewrite it and explain >>> the problem properly. >> >> The problem here is that the entry of struct ccp_cmd is not zeroed after we >> use it, >> If the other process got this address by kmalloc(), this illegal value >> "LIST_POISON1" >> will cause "general protection fault" error. > > If that's the case surely the other process should be zeroing > the memory? Your explanation still makes no sense. Yes, the other process should do this zero work, but the case I met is this address will appear in the slab_alloc_node() as freelist pointer of slub, and before slub do zero wrok, even kzalloc() doesn't work with this address. Thanks, Liwei. > > Thanks, >
Re: [PATCH] crypto: ccp - zero the cmd data after use it
On Tue, Aug 04, 2020 at 11:51:47AM +0800, Liwei Song wrote: > > On 8/3/20 20:52, Herbert Xu wrote: > > On Mon, Aug 03, 2020 at 03:58:58PM +0800, Liwei Song wrote: > >> exist the following assignment in ccp(ignore the force > >> convert of the struct) by list_del in ccp_dequeue_cmd(): > >> req->__ctx->cmd->entry->next = LIST_POISON1; > >> > >> after use the req, kzfree(req) can not zero the entry > >> entry->next = LIST_POISON1 of the ccp_cmd(cmd) struct > >> when this address available as slub freelist pointer, this will cause > >> the following "general protection fault" error if some process meet > >> this LIST_POISON1 value address when request memory: > > > > Your description makes no sense. Please rewrite it and explain > > the problem properly. > > The problem here is that the entry of struct ccp_cmd is not zeroed after we > use it, > If the other process got this address by kmalloc(), this illegal value > "LIST_POISON1" > will cause "general protection fault" error. If that's the case surely the other process should be zeroing the memory? Your explanation still makes no sense. Thanks, -- Email: Herbert Xu Home Page: http://gondor.apana.org.au/~herbert/ PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
Re: [PATCH] crypto: ccp - zero the cmd data after use it
On 8/3/20 20:52, Herbert Xu wrote: > On Mon, Aug 03, 2020 at 03:58:58PM +0800, Liwei Song wrote: >> exist the following assignment in ccp(ignore the force >> convert of the struct) by list_del in ccp_dequeue_cmd(): >> req->__ctx->cmd->entry->next = LIST_POISON1; >> >> after use the req, kzfree(req) can not zero the entry >> entry->next = LIST_POISON1 of the ccp_cmd(cmd) struct >> when this address available as slub freelist pointer, this will cause >> the following "general protection fault" error if some process meet >> this LIST_POISON1 value address when request memory: > > Your description makes no sense. Please rewrite it and explain > the problem properly. The problem here is that the entry of struct ccp_cmd is not zeroed after we use it, If the other process got this address by kmalloc(), this illegal value "LIST_POISON1" will cause "general protection fault" error. Thanks, Liwei. > > Thanks, >
Re: [PATCH] crypto: ccp - zero the cmd data after use it
On Mon, Aug 03, 2020 at 03:58:58PM +0800, Liwei Song wrote: > exist the following assignment in ccp(ignore the force > convert of the struct) by list_del in ccp_dequeue_cmd(): > req->__ctx->cmd->entry->next = LIST_POISON1; > > after use the req, kzfree(req) can not zero the entry > entry->next = LIST_POISON1 of the ccp_cmd(cmd) struct > when this address available as slub freelist pointer, this will cause > the following "general protection fault" error if some process meet > this LIST_POISON1 value address when request memory: Your description makes no sense. Please rewrite it and explain the problem properly. Thanks, -- Email: Herbert Xu Home Page: http://gondor.apana.org.au/~herbert/ PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
[PATCH] crypto: ccp - zero the cmd data after use it
exist the following assignment in ccp(ignore the force convert of the struct) by list_del in ccp_dequeue_cmd(): req->__ctx->cmd->entry->next = LIST_POISON1; after use the req, kzfree(req) can not zero the entry entry->next = LIST_POISON1 of the ccp_cmd(cmd) struct when this address available as slub freelist pointer, this will cause the following "general protection fault" error if some process meet this LIST_POISON1 value address when request memory: general protection fault: 1 PREEMPT SMP NOPTI CPU: 13 PID: 111282 Comm: msgstress03 Not tainted 5.2.45-yocto-standard #1 Hardware name: AMD Corporation Wallaby/Wallaby, BIOS WWB7713N 07/11/2017 RIP: 0010:__kmalloc_node+0x106/0x2f0 RSP: 0018:aa6dd83ffdc8 EFLAGS: 00010246 RAX: RBX: RCX: 0033e0cd RDX: 0033e08d RSI: 0033e08d RDI: 0002c180 RBP: aa6dd83ffe00 R08: 00d4 R09: 966c9dc07180 R10: dead0100 R11: R12: 0cc0 R13: 0100 R14: R15: 966c9dc07180 FS: 7f83bb756600() GS:966c9e34() knlGS: CS: 0010 DS: ES: CR0: 80050033 CR2: 7f83bb6917e0 CR3: 00080b794000 CR4: 003406e0 Call Trace: ? kvmalloc_node+0x7b/0x90 kvmalloc_node+0x7b/0x90 newque+0x32/0x1a0 ipcget+0x27a/0x2c0 ksys_msgget+0x51/0x70 __x64_sys_msgget+0x16/0x20 do_syscall_64+0x4d/0x130 entry_SYSCALL_64_after_hwframe+0x44/0xa9 RIP: 0033:0x7f83bb6917e7 Fix it by zero cmd struct after finished use it. Signed-off-by: Liwei Song --- drivers/crypto/ccp/ccp-dev.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/crypto/ccp/ccp-dev.c b/drivers/crypto/ccp/ccp-dev.c index edefa669153f..75a6418d541d 100644 --- a/drivers/crypto/ccp/ccp-dev.c +++ b/drivers/crypto/ccp/ccp-dev.c @@ -409,6 +409,7 @@ static void ccp_do_cmd_complete(unsigned long data) cmd->callback(cmd->data, cmd->ret); complete(>completion); + memset(cmd, 0, sizeof(*cmd)); } /** -- 2.17.1
