linux-toolchains.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Steven Rostedt <rostedt@goodmis.org>
To: Segher Boessenkool <segher@kernel.crashing.org>
Cc: Florian Weimer <fw@deneb.enyo.de>,
	Nick Desaulniers <ndesaulniers@google.com>,
	Peter Zijlstra <peterz@infradead.org>,
	Sami Tolvanen <samitolvanen@google.com>,
	Mathieu Desnoyers <mathieu.desnoyers@efficios.com>,
	linux-kernel <linux-kernel@vger.kernel.org>,
	Matt Mullins <mmullins@mmlx.us>, Ingo Molnar <mingo@redhat.com>,
	Alexei Starovoitov <ast@kernel.org>,
	Daniel Borkmann <daniel@iogearbox.net>,
	Dmitry Vyukov <dvyukov@google.com>,
	Martin KaFai Lau <kafai@fb.com>, Song Liu <songliubraving@fb.com>,
	Yonghong Song <yhs@fb.com>, Andrii Nakryiko <andriin@fb.com>,
	John Fastabend <john.fastabend@gmail.com>,
	KP Singh <kpsingh@chromium.org>, netdev <netdev@vger.kernel.org>,
	bpf <bpf@vger.kernel.org>, Kees Cook <keescook@chromium.org>,
	Josh Poimboeuf <jpoimboe@redhat.com>,
	linux-toolchains@vger.kernel.org
Subject: Re: violating function pointer signature
Date: Wed, 18 Nov 2020 14:33:43 -0500	[thread overview]
Message-ID: <20201118143343.4e86e79f@gandalf.local.home> (raw)
In-Reply-To: <20201118191127.GM2672@gate.crashing.org>

On Wed, 18 Nov 2020 13:11:27 -0600
Segher Boessenkool <segher@kernel.crashing.org> wrote:

> Calling this via a different declared function type is undefined
> behaviour, but that is independent of how the function is *defined*.
> Your program can make ducks appear from your nose even if that function
> is never called, if you do that.  Just don't do UB, not even once!

But that's the whole point of this conversation. We are going to call this
from functions that are going to have some random set of parameters.

But there is a limit to that. All the callers will expect a void return,
and none of the callers will have a variable number of parameters.

The code in question is tracepoints and static calls. For this
conversation, I'll stick with tracepoints (even though static calls are
used too, but including that in the conversation is confusing).

Let me define what is happening:

We have a macro that creates a defined tracepoint with a defined set of
parameters. But each tracepoint can have a different set of parameters. All
of them will have "void *" as the first parameter, but what comes after
that is unique to each tracepoint (defined by a macro). None of them will
be a variadic function call.

The macro looks like this:

	int __traceiter_##_name(void *__data, proto)			\
	{								\
		struct tracepoint_func *it_func_ptr;			\
		void *it_func;						\
									\
		it_func_ptr =						\
			rcu_dereference_raw((&__tracepoint_##_name)->funcs); \
		do {							\
			it_func = (it_func_ptr)->func;			\
			__data = (it_func_ptr)->data;			\
			((void(*)(void *, proto))(it_func))(__data, args); \
		} while ((++it_func_ptr)->func);			\
		return 0;						\
	}


There's an array of struct tracepoint_func pointers, which has the
definition of:

struct tracepoint_func {
	void *func;
	void *data;
	int prio;
};


And you see the above, the macro does:

	((void(*)(void *, proto))(it_func))(__data, args);

With it_func being the func from the struct tracepoint_func, which is a
void pointer, it is typecast to the function that is defined by the
tracepoint. args is defined as the arguments that match the proto.

The way the array is updated, is to use an RCU method, which is to create a
new array, copy the changes to the new array, then switch the "->funcs"
over to the new copy, and after a RCU grace period is finished, we can free
the old array.

The problem we are solving is on the removal case, if the memory is tight,
it is possible that the new array can not be allocated. But we must still
remove the called function. The idea in this case is to replace the
function saved with a stub. The above loop will call the stub and not the
removed function until another update happens.

This thread is about how safe is it to call:

void tp_stub_func(void) { return ; }

instead of the function that was removed?

Thus, we are indeed calling that stub function from a call site that is not
using the same parameters.

The question is, will this break?

-- Steve

  reply	other threads:[~2020-11-18 19:34 UTC|newest]

Thread overview: 33+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <20201116175107.02db396d@gandalf.local.home>
     [not found] ` <47463878.48157.1605640510560.JavaMail.zimbra@efficios.com>
     [not found]   ` <20201117142145.43194f1a@gandalf.local.home>
     [not found]     ` <375636043.48251.1605642440621.JavaMail.zimbra@efficios.com>
     [not found]       ` <20201117153451.3015c5c9@gandalf.local.home>
2020-11-18 13:21         ` violating function pointer signature Peter Zijlstra
2020-11-18 13:59           ` Florian Weimer
2020-11-18 14:12             ` Peter Zijlstra
2020-11-18 14:18               ` Florian Weimer
2020-11-18 14:34                 ` [PATCH v3] tracepoint: Do not fail unregistering a probe due to memory allocation Steven Rostedt
2020-11-24  5:59                   ` Matt Mullins
2020-11-18 14:22             ` violating function pointer signature Steven Rostedt
2020-11-18 19:46               ` Alexei Starovoitov
2020-11-18 20:02                 ` Steven Rostedt
2020-11-18 14:02           ` Steven Rostedt
2020-11-18 16:01             ` Mathieu Desnoyers
2020-11-18 16:19               ` David Laight
2020-11-18 16:50           ` Nick Desaulniers
2020-11-18 17:17             ` Steven Rostedt
2020-11-18 18:12               ` Segher Boessenkool
2020-11-18 18:31                 ` Florian Weimer
2020-11-18 18:55                   ` Segher Boessenkool
2020-11-18 18:58                   ` Steven Rostedt
2020-11-18 18:59                     ` Steven Rostedt
2020-11-18 19:11                     ` Segher Boessenkool
2020-11-18 19:33                       ` Steven Rostedt [this message]
2020-11-18 19:48                         ` Segher Boessenkool
2020-11-18 20:44                           ` Steven Rostedt
2020-11-19  8:21                           ` Peter Zijlstra
2020-11-19  8:36                       ` Peter Zijlstra
2020-11-19 14:37                         ` Segher Boessenkool
2020-11-19 14:59                           ` Steven Rostedt
2020-11-19 16:35                             ` Segher Boessenkool
2020-11-19 17:42                               ` David Laight
2020-11-19 19:27                                 ` Segher Boessenkool
2020-11-19 17:04                             ` Alexei Starovoitov
2020-11-19 17:30                               ` Steven Rostedt
2020-11-20  1:31                               ` Nick Desaulniers

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20201118143343.4e86e79f@gandalf.local.home \
    --to=rostedt@goodmis.org \
    --cc=andriin@fb.com \
    --cc=ast@kernel.org \
    --cc=bpf@vger.kernel.org \
    --cc=daniel@iogearbox.net \
    --cc=dvyukov@google.com \
    --cc=fw@deneb.enyo.de \
    --cc=john.fastabend@gmail.com \
    --cc=jpoimboe@redhat.com \
    --cc=kafai@fb.com \
    --cc=keescook@chromium.org \
    --cc=kpsingh@chromium.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-toolchains@vger.kernel.org \
    --cc=mathieu.desnoyers@efficios.com \
    --cc=mingo@redhat.com \
    --cc=mmullins@mmlx.us \
    --cc=ndesaulniers@google.com \
    --cc=netdev@vger.kernel.org \
    --cc=peterz@infradead.org \
    --cc=samitolvanen@google.com \
    --cc=segher@kernel.crashing.org \
    --cc=songliubraving@fb.com \
    --cc=yhs@fb.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).