On Wed, Jan 26, 2022 at 09:35:38AM +0900, Masami Hiramatsu wrote: > Hi Steve, > (Ccing Beau) > > On Tue, 25 Jan 2022 17:21:14 -0500 > Steven Rostedt <rostedt@xxxxxxxxxxx> wrote: > > > On Tue, 25 Jan 2022 14:07:14 -0800 > > Kees Cook <keescook@xxxxxxxxxxxx> wrote: > > > > > > The tstruct is the TP_STRUCT__entry() and for each __rel_dynamic_array() or > > > > __dynamic_array(), the __data_size gets updated and saved into the > > > > __data_offsets that holds where each item is. > > > > > > > > The rel versions sets the offset from its location to the data, where as > > > > the non rel versions sets the offset from the beginning of the event to the > > > > data. > > > > > > Could this just be > > > > > > #define __get_rel_dynamic_array(field) \ > > > ((void *)(&__entry->data[__entry->__rel_loc_##field & 0xffff]) > > > > > > ? > > > > This is currently user space defined. But since the only user of the rel_* > > version hasn't been upstreamed yet, we could change it. But it also > > requires changing libtraceevent as it depends on this code too. > > I think Kees' idea seems better. If you and Beau are good, I will update > the macros for __rel_loc. (This requires to change some user-space > application which Beau is making too.) > > > > > I'm surprised that it doesn't break with the __get_dynamic_array() > > versions, or is that because it's based off of __entry? > > I think so. Gcc seems to check the size of the data structure where the > original base address points. Right, and it's pretty good these days at navigating through casts and inlines, etc, which is appreciated in all the cases where it has found real bugs. :) The "offset from __entry" solution works as well as "offset from __entry->data", so I don't care which is used. If "offset from data" makes more sense for this API, yeah, I guess change it now while it's possible. :) Thanks for helping track this down! -- Kees Cook
