linux-kernel - Re: [PATCH v5 bpf-next 4/4] bpf: Support bpf_trampoline on functions with IPMODIFY (e.g. livepatch)

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: <C3E68D67-CC94-4BF0-90C5-BEF3E7784A21@fb.com>
Date:   Fri, 22 Jul 2022 15:58:44 +0000
From:   Song Liu <songliubraving@...com>
To:     Jiri Olsa <olsajiri@...il.com>
CC:     Song Liu <song@...nel.org>, bpf <bpf@...r.kernel.org>,
        lkml <linux-kernel@...r.kernel.org>,
        "live-patching@...r.kernel.org" <live-patching@...r.kernel.org>,
        Daniel Borkmann <daniel@...earbox.net>,
        Kernel Team <Kernel-team@...com>,
        "rostedt@...dmis.org" <rostedt@...dmis.org>
Subject: Re: [PATCH v5 bpf-next 4/4] bpf: Support bpf_trampoline on functions
 with IPMODIFY (e.g. livepatch)



> On Jul 22, 2022, at 12:31 AM, Jiri Olsa <olsajiri@...il.com> wrote:
> 
> On Tue, Jul 19, 2022 at 05:21:26PM -0700, Song Liu wrote:
> 
> SNIP
> 
>> +		tr->flags |= BPF_TRAMP_F_SHARE_IPMODIFY;
>> +
>> +		if ((tr->flags & BPF_TRAMP_F_CALL_ORIG) &&
>> +		    !(tr->flags & BPF_TRAMP_F_ORIG_STACK))
>> +			ret = bpf_trampoline_update(tr, false /* lock_direct_mutex */);
>> +		break;
>> +	case FTRACE_OPS_CMD_DISABLE_SHARE_IPMODIFY_PEER:
>> +		tr->flags &= ~BPF_TRAMP_F_SHARE_IPMODIFY;
>> +
>> +		if (tr->flags & BPF_TRAMP_F_ORIG_STACK)
>> +			ret = bpf_trampoline_update(tr, false /* lock_direct_mutex */);
>> +		break;
>> +	default:
>> +		ret = -EINVAL;
>> +		break;
>> +	};
>> +
>> +	mutex_unlock(&tr->mutex);
>> +	return ret;
>> +}
>> +#endif
>> +
>> bool bpf_prog_has_trampoline(const struct bpf_prog *prog)
>> {
>> 	enum bpf_attach_type eatype = prog->expected_attach_type;
>> @@ -89,6 +165,16 @@ static struct bpf_trampoline *bpf_trampoline_lookup(u64 key)
>> 	tr = kzalloc(sizeof(*tr), GFP_KERNEL);
>> 	if (!tr)
>> 		goto out;
>> +#ifdef CONFIG_DYNAMIC_FTRACE_WITH_DIRECT_CALLS
>> +	tr->fops = kzalloc(sizeof(struct ftrace_ops), GFP_KERNEL);
>> +	if (!tr->fops) {
>> +		kfree(tr);
>> +		tr = NULL;
>> +		goto out;
>> +	}
> 
> would it be easier to put ftrace_ops directly to bpf_trampoline,
> not just pointer.. it's allocated and freed at the same point
> 
> I recall there were some include issues when I tried that long
> time ago [1], but could make the change bit simpler
> 
> [1] https://git.kernel.org/pub/scm/linux/kernel/git/jolsa/perf.git/commit/?h=bpf/batch&id=52a1d4acdf55df41e99ca2cea51865e6821036ce

I was not aware the CC_USING_FENTRY issue. However, I think we
would like to avoid including ftrace.h in bpf.h, which will 
slow down the build. 

Thanks,
Song