lkml.org 
[lkml]   [2022]   [Sep]   [26]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [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

\
 
 \ /
  Last update: 2022-09-26 15:09    [W:0.036 / U:0.272 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site