[PATCH] slub: call BUG if next_object is not valid

[PATCH] slub: call BUG if next_object is not valid

[lijiazi]

If current object's memory is corrupted, there is a high
probability that next_objext stored in it will be rewritten as an
illegal value. It's better to check next_object this time than to
encounter a illegal pointer in next slub alloc like the following:

[80138.529667] Unable to handle kernel paging request at virtual
address 0069145a08d9a20d
[80138.529674] Mem abort info:
[80138.529677] ESR = 0x96000004
[80138.529683] Exception class = DABT (current EL), IL = 32 bits
[80138.529688] SET = 0, FnV = 0
[80138.529692] EA = 0, S1PTW = 0
[80138.529695] Data abort info:
[80138.529699] ISV = 0, ISS = 0x00000004
[80138.529703] CM = 0, WnR = 0
[80138.529708] [0069145a08d9a20d] address between user and kernel
address ranges
[80138.529716] Internal error: Oops: 96000004 1 PREEMPT SMP
[80138.529722] Modules linked in: wlan(O) rmnet_perf(O) rmnet_shs(O)
[80138.529812] CPU: 1 PID: 1074 Comm: cnss_diag Tainted: G S W O
4.19.72-perf-gdee6978 #1
[80138.529824] pstate: 60400005 (nZCv daif +PAN -UAO)
[80138.529840] pc : __kmalloc_track_caller+0x1d0/0x318
[80138.529845] lr : __kmalloc_track_caller+0x60/0x318
[80138.529849] sp : ffffff8011f6b980
[80138.529852] x29: ffffff8011f6b9e0 x28: ffffffa187f15248
[80138.529858] x27: ffffffede4856580 x26: ffffff8011f6bab8
[80138.529864] x25: ffffffa18a238000 x24: ffffffec8681f980
[80138.529870] x23: 2369145a08d9a20d x22: ffffffec8681f980
[80138.529877] x21: ffffffa188e8c964 x20: 00000000000001c0
[80138.529884] x19: 00000000007102c0 x18: 0000000000000000
[80138.529890] x17: 0000000000000000 x16: 0000000000000000
[80138.529897] x15: 0000007fffffffff x14: 0000000002a46f01
[80138.529903] x13: 0000000000000000 x12: ffffffee38964760
[80138.529909] x11: dc96ebb941026589 x10: 2369145a08d9a20d
[80138.529916] x9 : 0000000002a46ef9 x8 : ffffffede4856580
[80138.529922] x7 : 0000000000000000 x6 : 0000000000000004
[80138.529929] x5 : 0000000000000003 x4 : 00000000007000c0
[80138.529935] x3 : ffffff8011f6bba4 x2 : ffffffa188e8c964
[80138.529942] x1 : 00000000007102c0 x0 : 0000000000000000

[80138.530481] Call trace:
[80138.530488] __kmalloc_track_caller+0x1d0/0x318
[80138.530498] __alloc_skb+0x94/0x198
[80138.530504] alloc_skb_with_frags+0x5c/0x198
[80138.530511] sock_alloc_send_pskb+0x1d0/0x2c8
[80138.530520] unix_dgram_sendmsg+0x234/0xa80
[80138.530525] sock_write_iter+0xb8/0x110
[80138.530532] do_iter_readv_writev+0x118/0x158
[80138.530540] do_iter_write+0x7c/0x190
[80138.530544] vfs_writev+0x84/0xe8
[80138.530549] do_writev+0x78/0x118
[80138.530554] __arm64_sys_writev+0x1c/0x28
[80138.530564] el0_svc_common+0xa0/0x158
[80138.530569] el0_svc_handler+0x6c/0x88
[80138.530578] el0_svc+0x8/0xc

Signed-off-by: lijiazi <lijiazi@xiaomi.com>
---
 mm/slub.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/mm/slub.c b/mm/slub.c
index a0b335d..758e4e6 100644
--- a/mm/slub.c
+++ b/mm/slub.c
@@ -2744,6 +2744,9 @@ static __always_inline void *slab_alloc_node(struct kmem_cache *s,
 	} else {
 		void *next_object = get_freepointer_safe(s, object);
 
+		if (unlikely(!virt_addr_valid(next_object)))
+			BUG();
+
 		/*
 		 * The cmpxchg will only match if there was no additional
 		 * operation and if we are on the right processor.
-- 
2.7.4

Re: [PATCH] slub: call BUG if next_object is not valid

[Qian Cai]

> On Jan 3, 2020, at 6:17 AM, lijiazi <jqqlijiazi@gmail.com> wrote:
> 
> If current object's memory is corrupted, there is a high
> probability that next_objext stored in it will be rewritten as an
> illegal value. It's better to check next_object this time than to
> encounter a illegal pointer in next slub alloc like the following:

Rather than papering over the issue, the key to figure out is how was the current object memory corrupted?

Re: [PATCH] slub: call BUG if next_object is not valid

[Christopher Lameter]

On Fri, 3 Jan 2020, Qian Cai wrote:

> > On Jan 3, 2020, at 6:17 AM, lijiazi <jqqlijiazi@gmail.com> wrote:
> >
> > If current object's memory is corrupted, there is a high
> > probability that next_objext stored in it will be rewritten as an
> > illegal value. It's better to check next_object this time than to
> > encounter a illegal pointer in next slub alloc like the following:
>
> Rather than papering over the issue, the key to figure out is how was the current object memory corrupted?

Yes and this is a performance critical path. Keep expensive operations out
and enable them only if debugging is enabled.

Re: [PATCH] slub: call BUG if next_object is not valid

[David Hildenbrand]

On 03.01.20 15:49, Christopher Lameter wrote:
> On Fri, 3 Jan 2020, Qian Cai wrote:
> 
>>> On Jan 3, 2020, at 6:17 AM, lijiazi <jqqlijiazi@gmail.com> wrote:
>>>
>>> If current object's memory is corrupted, there is a high
>>> probability that next_objext stored in it will be rewritten as an
>>> illegal value. It's better to check next_object this time than to
>>> encounter a illegal pointer in next slub alloc like the following:
>>
>> Rather than papering over the issue, the key to figure out is how was the current object memory corrupted?
> 
> Yes and this is a performance critical path. Keep expensive operations out
> and enable them only if debugging is enabled.
> 

VM_BUG_ON(!virt_addr_valid(next_object));

But I agree that this might be misplaces, because we certainly don't
want to add random memory corruption checks in many call paths.

-- 
Thanks,

David / dhildenb