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.
> > }
> > memcpy(dst + curr_off, src + curr_off, map->value_size - curr_off);
> > }
> > @@ -335,16 +369,17 @@ static inline void zero_map_value(struct bpf_map *map, void *dst)
> > u32 curr_off = 0;
> > int i;
> >
> > - if (likely(!map->off_arr)) {
> > + if (likely(!map->field_offs)) {
> > memset(dst, 0, map->value_size);
> > 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;
> >
> > - memset(dst + curr_off, 0, next_off - curr_off);
> > - curr_off += map->off_arr->field_sz[i];
> > + memset(dst + curr_off, 0, sz);
> > + curr_off += map->field_offs->field_sz[i] + sz;
>
> same thing
