All of lore.kernel.org
 help / color / mirror / Atom feed
From: Kumar Kartikeya Dwivedi <memxor@gmail.com>
To: Alexei Starovoitov <alexei.starovoitov@gmail.com>
Cc: bpf@vger.kernel.org, Alexei Starovoitov <ast@kernel.org>,
	Andrii Nakryiko <andrii@kernel.org>,
	Daniel Borkmann <daniel@iogearbox.net>,
	Martin KaFai Lau <martin.lau@kernel.org>,
	Dave Marchevsky <davemarchevsky@meta.com>,
	Delyan Kratunov <delyank@meta.com>
Subject: Re: [PATCH bpf-next v4 06/24] bpf: Refactor kptr_off_tab into btf_record
Date: Fri, 4 Nov 2022 12:57:58 +0530	[thread overview]
Message-ID: <20221104072758.rr6uleke3arkpwxt@apollo> (raw)
In-Reply-To: <20221104070241.sa2v7ertneocowcq@apollo>

On Fri, Nov 04, 2022 at 12:32:41PM IST, Kumar Kartikeya Dwivedi wrote:
> On Fri, Nov 04, 2022 at 08:30:28AM IST, Alexei Starovoitov wrote:
> > On Fri, Nov 04, 2022 at 12:39:55AM +0530, Kumar Kartikeya Dwivedi wrote:
> > >  		else
> > > @@ -311,11 +344,12 @@ static inline void __copy_map_value(struct bpf_map *map, void *dst, void *src, b
> > >  		return;
> > >  	}
> > >
> > > -	for (i = 0; i < map->off_arr->cnt; i++) {
> > > -		u32 next_off = map->off_arr->field_off[i];
> > > +	for (i = 0; i < map->field_offs->cnt; i++) {
> > > +		u32 next_off = map->field_offs->field_off[i];
> > > +		u32 sz = next_off - curr_off;
> > >
> > > -		memcpy(dst + curr_off, src + curr_off, next_off - curr_off);
> > > -		curr_off += map->off_arr->field_sz[i];
> > > +		memcpy(dst + curr_off, src + curr_off, sz);
> > > +		curr_off += map->field_offs->field_sz[i] + sz;
> >
> > This is a clear bug. The kernel is crashing with this change.
> > How did you test this?
> >
>
> For me it is crashing at bpf-next now without this.
>
> When for map value with size 48, having fields at:
> off: 0, 16, 32
> sz:  4, 16, 16
>
> The above produces:
>
> memcpy(dst + 0, src + 0, 0)
> memcpy(dst + 4, src + 4, 12)
> memcpy(dst + 32, src + 32, 0)
> memcpy(dst + 48, src + 48, 0)
>
> Without it, it becomes:
>
> memcpy(dst + 0, src + 0, 0)
> memcpy(dst + 4, src + 4, 12)
> memcpy(dst + 20, src + 20, 12)
> memcpy(dst + 36, src + 36, 12)
>
> I will send a follow up fix.
>

I think this is broken in the original code, I didn't realise this at first but
it's effectively the same in that commit. So it should have Fixes: tag for that.

  reply	other threads:[~2022-11-04  7:28 UTC|newest]

Thread overview: 59+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-11-03 19:09 [PATCH bpf-next v4 00/24] Local kptrs, BPF linked lists Kumar Kartikeya Dwivedi
2022-11-03 19:09 ` [PATCH bpf-next v4 01/24] bpf: Document UAPI details for special BPF types Kumar Kartikeya Dwivedi
2022-11-03 20:38   ` David Vernet
2022-11-03 19:09 ` [PATCH bpf-next v4 02/24] bpf: Allow specifying volatile type modifier for kptrs Kumar Kartikeya Dwivedi
2022-11-03 20:45   ` David Vernet
2022-11-03 19:09 ` [PATCH bpf-next v4 03/24] bpf: Clobber stack slot when writing over spilled PTR_TO_BTF_ID Kumar Kartikeya Dwivedi
2022-11-03 19:09 ` [PATCH bpf-next v4 04/24] bpf: Fix slot type check in check_stack_write_var_off Kumar Kartikeya Dwivedi
2022-11-03 19:09 ` [PATCH bpf-next v4 05/24] bpf: Drop reg_type_may_be_refcounted_or_null Kumar Kartikeya Dwivedi
2022-11-03 21:55   ` David Vernet
2022-11-03 19:09 ` [PATCH bpf-next v4 06/24] bpf: Refactor kptr_off_tab into btf_record Kumar Kartikeya Dwivedi
2022-11-04  2:44   ` Alexei Starovoitov
2022-11-04  3:00   ` Alexei Starovoitov
2022-11-04  7:02     ` Kumar Kartikeya Dwivedi
2022-11-04  7:27       ` Kumar Kartikeya Dwivedi [this message]
2022-11-04  3:16   ` Alexei Starovoitov
2022-11-04  4:00   ` Alexei Starovoitov
2022-11-04  4:09     ` Alexei Starovoitov
2022-11-04  7:34       ` Kumar Kartikeya Dwivedi
2022-11-03 19:09 ` [PATCH bpf-next v4 07/24] bpf: Consolidate spin_lock, timer management " Kumar Kartikeya Dwivedi
2022-11-04  4:52   ` Alexei Starovoitov
2022-11-04  5:30   ` Alexei Starovoitov
2022-11-04  6:43     ` Kumar Kartikeya Dwivedi
2022-11-04  6:47       ` Kumar Kartikeya Dwivedi
2022-11-03 19:09 ` [PATCH bpf-next v4 08/24] bpf: Refactor map->off_arr handling Kumar Kartikeya Dwivedi
2022-11-03 19:09 ` [PATCH bpf-next v4 09/24] bpf: Support bpf_list_head in map values Kumar Kartikeya Dwivedi
2022-11-03 19:09 ` [PATCH bpf-next v4 10/24] bpf: Introduce local kptrs Kumar Kartikeya Dwivedi
2022-11-04  5:57   ` Alexei Starovoitov
2022-11-04  7:51     ` Kumar Kartikeya Dwivedi
2022-11-04 15:38       ` Alexei Starovoitov
2022-11-05  2:19       ` Dave Marchevsky
2022-11-03 19:10 ` [PATCH bpf-next v4 11/24] bpf: Recognize bpf_{spin_lock,list_head,list_node} in " Kumar Kartikeya Dwivedi
2022-11-03 19:10 ` [PATCH bpf-next v4 12/24] bpf: Verify ownership relationships for user BTF types Kumar Kartikeya Dwivedi
2022-11-03 19:10 ` [PATCH bpf-next v4 13/24] bpf: Support locking bpf_spin_lock in local kptr Kumar Kartikeya Dwivedi
2022-11-03 19:10 ` [PATCH bpf-next v4 14/24] bpf: Allow locking bpf_spin_lock global variables Kumar Kartikeya Dwivedi
2022-11-04  2:54   ` Dave Marchevsky
2022-11-04  7:56     ` Kumar Kartikeya Dwivedi
2022-11-03 19:10 ` [PATCH bpf-next v4 15/24] bpf: Rewrite kfunc argument handling Kumar Kartikeya Dwivedi
2022-11-03 19:10 ` [PATCH bpf-next v4 16/24] bpf: Drop kfunc bits from btf_check_func_arg_match Kumar Kartikeya Dwivedi
2022-11-03 19:10 ` [PATCH bpf-next v4 17/24] bpf: Support constant scalar arguments for kfuncs Kumar Kartikeya Dwivedi
2022-11-03 19:10 ` [PATCH bpf-next v4 18/24] bpf: Teach verifier about non-size constant arguments Kumar Kartikeya Dwivedi
2022-11-03 19:10 ` [PATCH bpf-next v4 19/24] bpf: Introduce bpf_obj_new Kumar Kartikeya Dwivedi
2022-11-04  2:37   ` Dave Marchevsky
2022-11-04  8:09     ` Kumar Kartikeya Dwivedi
2022-11-04 15:39       ` Alexei Starovoitov
2022-11-03 19:10 ` [PATCH bpf-next v4 20/24] bpf: Introduce bpf_obj_drop Kumar Kartikeya Dwivedi
2022-11-03 19:10 ` [PATCH bpf-next v4 21/24] bpf: Permit NULL checking pointer with non-zero fixed offset Kumar Kartikeya Dwivedi
2022-11-03 19:10 ` [PATCH bpf-next v4 22/24] bpf: Introduce single ownership BPF linked list API Kumar Kartikeya Dwivedi
2022-11-04  5:56   ` Dave Marchevsky
2022-11-04  7:42     ` Kumar Kartikeya Dwivedi
2022-11-05  2:15       ` Dave Marchevsky
2022-11-05 18:16         ` Alexei Starovoitov
2022-11-06  1:53           ` Kumar Kartikeya Dwivedi
2022-11-03 19:10 ` [PATCH bpf-next v4 23/24] selftests/bpf: Add __contains macro to bpf_experimental.h Kumar Kartikeya Dwivedi
2022-11-03 19:10 ` [PATCH bpf-next v4 24/24] selftests/bpf: Add BPF linked list API tests Kumar Kartikeya Dwivedi
2022-11-04  7:03   ` Dave Marchevsky
2022-11-04  7:14     ` Kumar Kartikeya Dwivedi
2022-11-04  5:00 ` [PATCH bpf-next v4 00/24] Local kptrs, BPF linked lists patchwork-bot+netdevbpf
2022-11-04  5:30 ` patchwork-bot+netdevbpf
2022-11-04  6:30 ` patchwork-bot+netdevbpf

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=20221104072758.rr6uleke3arkpwxt@apollo \
    --to=memxor@gmail.com \
    --cc=alexei.starovoitov@gmail.com \
    --cc=andrii@kernel.org \
    --cc=ast@kernel.org \
    --cc=bpf@vger.kernel.org \
    --cc=daniel@iogearbox.net \
    --cc=davemarchevsky@meta.com \
    --cc=delyank@meta.com \
    --cc=martin.lau@kernel.org \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.