From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-pf0-f198.google.com (mail-pf0-f198.google.com [209.85.192.198]) by kanga.kvack.org (Postfix) with ESMTP id 72C146B0038 for ; Mon, 20 Nov 2017 03:42:18 -0500 (EST) Received: by mail-pf0-f198.google.com with SMTP id 82so8379969pfp.5 for ; Mon, 20 Nov 2017 00:42:18 -0800 (PST) Received: from mail-sor-f65.google.com (mail-sor-f65.google.com. [209.85.220.65]) by mx.google.com with SMTPS id k9sor646299pgo.347.2017.11.20.00.42.17 for (Google Transport Security); Mon, 20 Nov 2017 00:42:17 -0800 (PST) MIME-Version: 1.0 In-Reply-To: <20171120015000.GA13507@js1304-P5Q-DELUXE> References: <20171117223043.7277-1-wen.gang.wang@oracle.com> <20171120015000.GA13507@js1304-P5Q-DELUXE> From: Dmitry Vyukov Date: Mon, 20 Nov 2017 09:41:55 +0100 Message-ID: Subject: Re: [PATCH 0/5] mm/kasan: advanced check Content-Type: text/plain; charset="UTF-8" Sender: owner-linux-mm@kvack.org List-ID: To: Joonsoo Kim Cc: Wengang Wang , Linux-MM , Andrey Ryabinin , Alexander Potapenko , kasan-dev On Mon, Nov 20, 2017 at 2:50 AM, Joonsoo Kim wrote: > On Fri, Nov 17, 2017 at 11:56:21PM +0100, Dmitry Vyukov wrote: >> On Fri, Nov 17, 2017 at 11:30 PM, Wengang Wang wrote: >> > Kasan advanced check, I'm going to add this feature. >> > Currently Kasan provide the detection of use-after-free and out-of-bounds >> > problems. It is not able to find the overwrite-on-allocated-memory issue. >> > We sometimes hit this kind of issue: We have a messed up structure >> > (usually dynamially allocated), some of the fields in the structure were >> > overwritten with unreasaonable values. And kernel may panic due to those >> > overeritten values. We know those fields were overwritten somehow, but we >> > have no easy way to find out which path did the overwritten. The advanced >> > check wants to help in this scenario. >> > >> > The idea is to define the memory owner. When write accesses come from >> > non-owner, error should be reported. Normally the write accesses on a given >> > structure happen in only several or a dozen of functions if the structure >> > is not that complicated. We call those functions "allowed functions". >> > The work of defining the owner and binding memory to owner is expected to >> > be done by the memory consumer. In the above case, memory consume register >> > the owner as the functions which have write accesses to the structure then >> > bind all the structures to the owner. Then kasan will do the "owner check" >> > after the basic checks. >> > >> > As implementation, kasan provides a API to it's user to register their >> > allowed functions. The API returns a token to users. At run time, users >> > bind the memory ranges they are interested in to the check they registered. >> > Kasan then checks the bound memory ranges with the allowed functions. >> > >> > >> > Signed-off-by: Wengang Wang > > Hello, Wengang. > > Nice idea. I also think that we need this kind of debugging tool. It's very > hard to detect overwritten bugs. > > In fact, I made a quite similar tool, valid access checker (A.K.A. > vchecker). See the following link. > > https://github.com/JoonsooKim/linux/tree/vchecker-master-v0.3-next-20170106 > > Vchecker has some advanced features compared to yours. > > 1. Target object can be choosen at runtime by debugfs. It doesn't > require re-compile to register the target object. > > 2. It has another feature that checks the value stored in the object. > Usually, invalid writer stores odd value into the object and vchecker > can detect this case. > > 3. It has a callstack checker (memory owner checker in yours). It > checks all the callstack rather than just the caller. It's important > since invalid writer could call the parent function of owner function > and it would not be catched by checking just the caller. > > 4. The callstack checker is more automated. vchecker collects the valid > callstack by running the system. > > > FYI, I attach some commit descriptions of the vchecker. > > vchecker: store/report callstack of value writer > > The purpose of the value checker is finding invalid user writing > invalid value at the moment that the value is written. However, there is > a missing infrastructure that passes writing value to the checker > since we temporarilly piggyback on the KASAN. So, we cannot easily > detect this case in time. > > However, by following way, we can emulate similar effect. > > 1. Store callstack when memory is written. > 2. If check is failed in next access, report previous write-access > callstack > > It will caught offending user properly. > > > Following output "Call trace: Invalid writer" part is the result > of this patch. We find the invalid value at workfn+0x71 but report > writer at workfn+0x61. > > [ 133.024076] ================================================================== > [ 133.025576] BUG: VCHECKER: invalid access in workfn+0x71/0xc0 at addr ffff8800683dd6c8 > [ 133.027196] Read of size 8 by task kworker/1:1/48 > [ 133.028020] 0x8 0x10 value > [ 133.028020] 0xffff 4 > [ 133.028020] Call trace: Invalid writer > [ 133.028020] > [ 133.028020] [] save_stack_trace+0x1b/0x20 > [ 133.028020] > [ 133.028020] [] save_stack+0x39/0x70 > [ 133.028020] > [ 133.028020] [] check_value+0x43/0x80 > [ 133.028020] > [ 133.028020] [] vchecker_check+0x1c2/0x380 > [ 133.028020] > [ 133.028020] [] __asan_store8+0x8d/0xc0 > [ 133.028020] > [ 133.028020] [] workfn+0x61/0xc0 > [ 133.028020] > [ 133.028020] [] process_one_work+0x28f/0x680 > [ 133.028020] > [ 133.028020] [] worker_thread+0xa2/0x870 > [ 133.028020] > [ 133.028020] [] kthread+0x195/0x1e0 > [ 133.028020] > [ 133.028020] [] ret_from_fork+0x22/0x30 > [ 133.028020] CPU: 1 PID: 48 Comm: kworker/1:1 Not tainted 4.10.0-rc2-next-20170106+ #1179 > [ 133.028020] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Bochs 01/01/2011 > [ 133.028020] Workqueue: events workfn > [ 133.028020] Call Trace: > [ 133.028020] dump_stack+0x4d/0x63 > [ 133.028020] kasan_object_err+0x21/0x80 > [ 133.028020] vchecker_check+0x2af/0x380 > [ 133.028020] ? workfn+0x71/0xc0 > [ 133.028020] ? workfn+0x71/0xc0 > [ 133.028020] __asan_load8+0x87/0xb0 > [ 133.028020] workfn+0x71/0xc0 > [ 133.028020] process_one_work+0x28f/0x680 > [ 133.028020] worker_thread+0xa2/0x870 > [ 133.028020] kthread+0x195/0x1e0 > [ 133.028020] ? put_pwq_unlocked+0xc0/0xc0 > [ 133.028020] ? kthread_park+0xd0/0xd0 > [ 133.028020] ret_from_fork+0x22/0x30 > [ 133.028020] Object at ffff8800683dd6c0, in cache vchecker_test size: 24 > [ 133.028020] Allocated: > [ 133.028020] PID = 48 > > > vchecker: Add 'callstack' checker > > The callstack checker is to find invalid code paths accessing to a > certain field in an object. Currently it only saves all stack traces at > the given offset. Reporting will be added in the next patch. > > The below example checks callstack of anon_vma: > > # cd /sys/kernel/debug/vchecker > # echo 0 8 > anon_vma/callstack # offset 0, size 8 > # echo 1 > anon_vma/enable > > # cat anon_vma/callstack # show saved callstacks > 0x0 0x8 callstack > total: 42 > callstack #0 > anon_vma_fork+0x101/0x280 > copy_process.part.10+0x15ff/0x2a40 > _do_fork+0x155/0x7d0 > SyS_clone+0x19/0x20 > do_syscall_64+0xdf/0x460 > return_from_SYSCALL_64+0x0/0x7a > ... > > > vchecker: Support toggle on/off of callstack check > > By default, callstack checker only collects callchains. When a user > writes 'on' to the callstack file in debugfs, it checks and reports new > callstacks. Writing 'off' to disable it again. > > # cd /sys/kernel/debug/vchecker > # echo 0 8 > anon_vma/callstack > # echo 1 > anon_vma/enable > > ... (do some work to collect enough callstacks) ... > > # echo on > anon_vma/callstack > > > The reason I didn't submit the vchecker to mainline is that I didn't find > the case that this tool is useful in real life. Most of the system broken case > can be debugged by other ways. Do you see the real case that this tool is > helpful? Hi, Yes, this is the main question here. How is it going to be used in real life? How widely? > If so, I think that vchecker is more appropriate to be upstreamed. > Could you share your opinion? > > Thanks. -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to majordomo@kvack.org. For more info on Linux MM, see: http://www.linux-mm.org/ . Don't email: email@kvack.org