On Mon, Nov 20, 2017 at 2:50 AM, Joonsoo Kim <iamjoonsoo.kim@xxxxxxx> 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 <wen.gang.wang@xxxxxxxxxx> 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 <wen.gang.wang@xxxxxxxxxx> > > 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] [<ffffffff81043b1b>] save_stack_trace+0x1b/0x20 > [ 133.028020] > [ 133.028020] [<ffffffff812c0db9>] save_stack+0x39/0x70 > [ 133.028020] > [ 133.028020] [<ffffffff812c0fe3>] check_value+0x43/0x80 > [ 133.028020] > [ 133.028020] [<ffffffff812c1762>] vchecker_check+0x1c2/0x380 > [ 133.028020] > [ 133.028020] [<ffffffff812be49d>] __asan_store8+0x8d/0xc0 > [ 133.028020] > [ 133.028020] [<ffffffff815eadd1>] workfn+0x61/0xc0 > [ 133.028020] > [ 133.028020] [<ffffffff810be3df>] process_one_work+0x28f/0x680 > [ 133.028020] > [ 133.028020] [<ffffffff810bf272>] worker_thread+0xa2/0x870 > [ 133.028020] > [ 133.028020] [<ffffffff810c86a5>] kthread+0x195/0x1e0 > [ 133.028020] > [ 133.028020] [<ffffffff81b9d3d2>] 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@xxxxxxxxx. For more info on Linux MM, see: http://www.linux-mm.org/ . Don't email: <a href=mailto:"dont@xxxxxxxxx";> email@xxxxxxxxx </a>
