Messages in this thread |  | | Date | Mon, 26 Sep 2022 19:08:07 +0800 | From | Kassey Li <> | Subject | Re: [PATCH v2] cgroup: align the comm length with TASK_COMM_LEN |
| |
On 9/26/2022 10:42 AM, Steven Rostedt wrote: > On Mon, 26 Sep 2022 10:18:55 +0800 > Kassey Li <quic_yingangl@quicinc.com> wrote: > >>>> @@ -139,12 +139,12 @@ DECLARE_EVENT_CLASS(cgroup_migrate, >>>> __entry->dst_level = dst_cgrp->level; >>>> __assign_str(dst_path, path); >>>> __entry->pid = task->pid; >>>> - __assign_str(comm, task->comm); >>>> + memcpy(__entry->comm, task->comm, TASK_COMM_LEN); >> I think the problem is here, __assign_str using strcpy >> the task->comm here tail is not '\0' >> that's why it out of bounds access. >> > > If this is the case, then there's a lot more than just tracing that will > break. There are other places in the kernel has used strcpy() on task->comm, > and many more that do "%s" on task->comm, which would also crash on this.
You are right.
by re-check my local logs(arm64), we can see the src has '\0' as end of string. but looks strcpy did not catch this and crossed. I can not figure out how this could happen. if there is debug suggest, please help.
src: task->comm SharedPreferenc pid 28395 _____________________address|________0________4________8________C_0123456789ABCDEF NSD:0000::FFFFFFBD1B6C59D0|>72616853 72506465 72656665 00636E65 SharedPreferenc.
dst: trace event buffer: _____________________address|________0________4________8________C_0123456789ABCDEF NSD:0000::FFFFFFBCF744FFE0| 00090020 000B0029 706F742F 7070612D ...).../top-app NSD:0000::FFFFFFBCF744FFF0| 61685300 50646572 65666572 636E6572 .SharedPreferenc NSD:0000::FFFFFFBCF7450000|>52800101 97FD3A05 140000B3 AA1303E0 ...R.:..........
layout of the struct:
[ND:0x0::0xFFFFFFBCF744FFC8] (struct trace_event_raw_cgroup_migrate)0xFFFFFFBCF744FFc8 = ( [ND:0x0::0xFFFFFFBCF744FFC8] ent = ( [ND:0x0::0xFFFFFFBCF744FFC8] type = 0x98, [ND:0x0::0xFFFFFFBCF744FFCA] flags = 0x1, [ND:0x0::0xFFFFFFBCF744FFCB] preempt_count = 0x1, [ND:0x0::0xFFFFFFBCF744FFCC] pid = 0x0773), [ND:0x0::0xFFFFFFBCF744FFD0] dst_root = 0x1, [ND:0x0::0xFFFFFFBCF744FFD4] dst_id = 0x6, [ND:0x0::0xFFFFFFBCF744FFD8] dst_level = 0x1, [ND:0x0::0xFFFFFFBCF744FFDC] pid = 28395 = 0x6EEB, [ND:0x0::0xFFFFFFBCF744FFE0] __data_loc_dst_path = 0x00090020 = '... ', [ND:0x0::0xFFFFFFBCF744FFE4] __data_loc_comm = 0x000B0029 = '...)', [ND:0x0::0xFFFFFFBCF744FFE8] __data_=_"/top-app")
name: cgroup_attach_task ID: 152 format: field:unsigned short common_type; offset:0; size:2; signed:0; field:unsigned char common_flags; offset:2; size:1; signed:0; field:unsigned char common_preempt_count; offset:3; size:1; signed:0; field:int common_pid; offset:4; size:4; signed:1;
field:int dst_root; offset:8; size:4; signed:1; field:int dst_id; offset:12; size:4; signed:1; field:int dst_level; offset:16; size:4; signed:1; field:int pid; offset:20; size:4; signed:1; field:__data_loc char[] dst_path; offset:24; size:4; signed:0; field:__data_loc char[] comm; offset:28; size:4; signed:0;
_____________________address|________0________4________8________C_0123456789ABCDEF NSD:0000::FFFFFFBCF744FFC0| 00656C64 0066D18D>01010098 00000773 dle...f.....s... NSD:0000::FFFFFFBCF744FFD0| 00000001 00000006 00000001 00006EEB .............n.. NSD:0000::FFFFFFBCF744FFE0| 00090020 000B0029 706F742F 7070612D ...).../top-app NSD:0000::FFFFFFBCF744FFF0| 61685300 50646572 65666572 636E6572 .SharedPreferenc NSD:0000::FFFFFFBCF7450000| 52800101 97FD3A05 140000B3 AA1303E0 ...R.:..........
> >> do you want to this version or just modify the memcpy or strncpy to do >> with a known length ? please give suggest so I can modify . > > I'm guessing a problem exists elsewhere that makes it look like this is the > issue. I suggest finding where the '\0' is dropped (if that is indeed the > case). > > -- Steve
|  |