Subject: Re: [PATCH v10 1/5] add infrastructure for tagging functions as error
 injectable
To: Masami Hiramatsu <mhiramat@kernel.org>
References: <1513365176-6744-1-git-send-email-josef@toxicpanda.com>
 <1513365176-6744-2-git-send-email-josef@toxicpanda.com>
 <20171219152925.5789309c6c4d27807d42f11c@kernel.org>
 <7b73cbfa-959f-2efe-8a7c-f9d0b0c2ccaa@fb.com>
 <20171220161342.44443047f186b7a62efdfe1a@kernel.org>
CC: Josef Bacik <josef@toxicpanda.com>, <rostedt@goodmis.org>,
        <mingo@redhat.com>, <davem@davemloft.net>, <netdev@vger.kernel.org>,
        <linux-kernel@vger.kernel.org>, <ast@kernel.org>, <kernel-team@fb.com>,
        <daniel@iogearbox.net>, <linux-btrfs@vger.kernel.org>,
        <darrick.wong@oracle.com>, Josef Bacik <jbacik@fb.com>
From: Alexei Starovoitov <ast@fb.com>
Message-ID: <5402c8bd-149c-04c2-1e7c-143559c35fe3@fb.com>
Date: Wed, 20 Dec 2017 09:22:00 -0800
User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.12; rv:45.0)
 Gecko/20100101 Thunderbird/45.8.0
MIME-Version: 1.0
In-Reply-To: <20171220161342.44443047f186b7a62efdfe1a@kernel.org>
Content-Type: text/plain; charset="windows-1252"; format=flowed
Content-Transfer-Encoding: 7bit
SpamDiagnosticOutput: 1:99
SpamDiagnosticMetadata: NSPM
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 20 Dec 2017 17:22:04.1313 (UTC)
X-MS-Exchange-CrossTenant-Network-Message-Id: ce959b86-685d-426e-b331-08d547ce3146
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 8ae927fe-1255-47a7-a2af-5f3a069daaa2
X-MS-Exchange-Transport-CrossTenantHeadersStamped: CO1PR15MB0966
X-OriginatorOrg: fb.com
X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10432:,, definitions=2017-12-20_07:,,
 signatures=0
X-Proofpoint-Spam-Reason: safe
X-FB-Internal: Safe
Sender: linux-kernel-owner@vger.kernel.org
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org
Content-Length: 1765
Lines: 45

On 12/19/17 11:13 PM, Masami Hiramatsu wrote:
> On Tue, 19 Dec 2017 18:14:17 -0800
> Alexei Starovoitov <ast@fb.com> wrote:
>
>> On 12/18/17 10:29 PM, Masami Hiramatsu wrote:
>>>>
>>>> +#if defined(__KERNEL__) && !defined(__ASSEMBLY__)
>>>> +#ifdef CONFIG_BPF_KPROBE_OVERRIDE
>>>
>>> BTW, CONFIG_BPF_KPROBE_OVERRIDE is also confusable name.
>>> Since this feature override a function to just return with
>>> some return value (as far as I understand, or would you
>>> also plan to modify execution path inside a function?),
>>> I think it should be better CONFIG_BPF_FUNCTION_OVERRIDE or
>>> CONFIG_BPF_EXECUTION_OVERRIDE.
>>
>> I don't think such renaming makes sense.
>> The feature is overriding kprobe by changing how kprobe returns.
>> It doesn't override BPF_FUNCTION or BPF_EXECUTION.
>
> No, I meant this is BPF's feature which override FUNCTION, so
> BPF is a kind of namespace. (Is that only for a function entry
> because it can not tweak stackframe at this morment?)
>
>> The kernel enters and exists bpf program as normal.
>
> Yeah, but that bpf program modifies instruction pointer, am I correct?

no. bpf side is asking kprobe side to modify it.
bpf cannot do such things as modifying IP or any other register
directly.

>>
>>> Indeed, BPF is based on kprobes, but it seems you are limiting it
>>> with ftrace (function-call trace) (I'm not sure the reason why),
>>> so using "kprobes" for this feature seems strange for me.
>>
>> do you have an idea how kprobe override can happen when kprobe
>> placed in the middle of the function?
>
> For example, if you know a basic block in the function, maybe
> you can skip a block or something like that. But nowadays
> it is somewhat hard because optimizer mixed it up.

still missing how that can work...