linux-kernel - Re: linux-next: build failure after merge of the kspp tree

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: <202201251914.9377C63D16@keescook>
Date:   Tue, 25 Jan 2022 19:17:32 -0800
From:   Kees Cook <keescook@...omium.org>
To:     Masami Hiramatsu <mhiramat@...nel.org>
Cc:     Steven Rostedt <rostedt@...dmis.org>,
        Stephen Rothwell <sfr@...b.auug.org.au>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        Linux Next Mailing List <linux-next@...r.kernel.org>,
        Beau Belgrave <beaub@...ux.microsoft.com>
Subject: Re: linux-next: build failure after merge of the kspp tree

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@...dmis.org> wrote:
> 
> > On Tue, 25 Jan 2022 14:07:14 -0800
> > Kees Cook <keescook@...omium.org> 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