linux-kernel - Re: [PATCH v10 2/4] trace/objtrace: Get the value of the object

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-Id: <20220522232216.bb1451fb6efc18c2bccc8d09@kernel.org>
Date:   Sun, 22 May 2022 23:22:16 +0900
From:   Masami Hiramatsu (Google) <mhiramat@...nel.org>
To:     Jeff Xie <xiehuan09@...il.com>
Cc:     rostedt@...dmis.org, mingo@...hat.com, mhiramat@...nel.org,
        zanussi@...nel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v10 2/4] trace/objtrace: Get the value of the object

Hi Jeff,

On Fri, 13 May 2022 01:00:06 +0800
Jeff Xie <xiehuan09@...il.com> wrote:

[...]
> @@ -175,9 +271,27 @@ trace_object_trigger(struct event_trigger_data *data,
>  
>  	field = obj_data->field;
>  	memcpy(&obj, rec + field->offset, sizeof(obj));
> -	set_trace_object(obj, tr);
> +	/* set the offset from the special object and the type size of the value*/
> +	set_trace_object(obj, obj_data->obj_offset,
> +			obj_data->obj_value_type_size, tr);
>  }
>  
> +static const struct objtrace_fetch_type objtrace_fetch_types[] = {
> +	{"u8", 1},
> +	{"s8", 1},
> +	{"x8", 1},
> +	{"u16", 2},
> +	{"s16", 2},
> +	{"x16", 2},
> +	{"u32", 4},
> +	{"s32", 4},
> +	{"x32", 4},
> +	{"u64", 8},
> +	{"s64", 8},
> +	{"x64", 8},

Hmm, as far as I can see, you don't distinguish the prefix 'u','s','x'.
If so, please support only 'x' at this moment. kprobe events supports
those types, and it distinguishes the types when printing the logged
data. E.g. 's16' shows '-1' for 0xffff, but 'x16' shows '0xffff'.
You can add another patch to support such different types afterwards.

> +	{}

If this array is null terminated, please explictly do that, like

	{NULL, 0},

for readability.

Thank you,

-- 
Masami Hiramatsu (Google) <mhiramat@...nel.org>