Re: [PATCH bpf v5 1/2] bpf: Fix OOB in pcpu_init_value

[Martin KaFai Lau <martin.lau@linux.dev>]

On Thu, Apr 02, 2026 at 05:05:14PM -0700, Alexei Starovoitov wrote:
> On Thu, Apr 2, 2026 at 12:59 PM Martin KaFai Lau <martin.lau@linux.dev> wrote:
> >
> > On Thu, Apr 02, 2026 at 11:36:04AM -0700, Alexei Starovoitov wrote:
> > > On Thu, Apr 2, 2026 at 10:01 AM Martin KaFai Lau <martin.lau@linux.dev> wrote:
> > > >
> > > > On Thu, Apr 02, 2026 at 07:17:17AM -0700, Alexei Starovoitov wrote:
> > > > > On Thu, Apr 2, 2026 at 12:43 AM xulang <xulang@uniontech.com> wrote:
> > > > > >
> > > > > > From: Lang Xu <xulang@uniontech.com>
> > > > > >
> > > > > > An out-of-bounds read occurs when copying element from a
> > > > > > BPF_MAP_TYPE_CGROUP_STORAGE map to another pcpu map with the
> > > > > > same value_size that is not rounded up to 8 bytes.
> > > > > >
> > > > > > The issue happens when:
> > > > > > 1. A CGROUP_STORAGE map is created with value_size not aligned to
> > > > > >    8 bytes (e.g., 4 bytes)
> > > > > > 2. A pcpu map is created with the same value_size (e.g., 4 bytes)
> > > > > > 3. Update element in 2 with data in 1
> > > > > >
> > > > > > pcpu_init_value assumes that all sources are rounded up to 8 bytes,
> > > > > > and invokes copy_map_value_long to make a data copy, However, the
> > > > > > assumption doesn't stand since there are some cases where the source
> > > > > > may not be rounded up to 8 bytes, e.g., CGROUP_STORAGE,
> > > > >
> > > > > why? Just round it up there instead of penalizing perf everywhere.
> > > > >
> > > > > > skb->data.
> > > > >
> > > > > what that means?
> > > > >
> > > > > pcpu_init_value() can access skb->data ?
> > > >
> > > > After bound check, the skb->data can be used in
> > > > bpf_map_update_elem(&percpu_lru_map, &key, skb_data, BPF_NOEXIST)
> > > > which will call pcpu_init_value().
> > >
> > > I see, but if we round up on cgroup storage size the problem is gone,
> > > right?
> >
> > Right, it will fix the problem tested in patch 2 which
> > passes cgroup_storage_value as the source to
> > pcpu_init_value(). The bug should only manifest with BPF_NOEXIST.
> > For BPF_EXIST, pcpu_copy_value() will be used and it
> > currently uses copy_map_value() instead of copy_map_value_long().
> >
> > > Doesn't matter what the source of the copy is.
> >
> > I think the source (PTR_TO_*) matters here because the bug is about
> > reading beyond the boundary of the source. A few other map types
> > were audited when their values were used as the source.
> >
> > For skb->data, using skb->data to reproduce is practically
> > not possible because there should be at least shinfo beyond data_end,
> > so some of shinfo may get copied to the pcpu map in the extreme case.
> 
> Yes, but also the verifier checks that ptr + value_size is accessible
> in that source. In this case, that the value_size bytes are available in skb.
> So if we round up early at cgroup storage creation time there is no overrun.

The verifier checks that src_ptr + value_size is accessible but the
percpu's map->value_size is not rounded up to 8 bytes. If the percpu_map
is created with 4 bytes value_size, the percpu_map->value_size will stay at 4
and verifier only checks that src + 4 is accessible.
For example:

	if (data + 14 > data_end)
		return TC_ACT_SHOT;

	src = data + 10;
	/* percpu_lru_map.value_size is 4 */
	update_ret = bpf_map_update_elem(&percpu_lru_map, &key, src, BPF_NOEXIST);

The above prog can be loaded but src is only 4-byte accessible instead of 8.
It happens to work for skb because there is always a shinfo at the end.
My concern is other source ptr types such as PTR_TO_STACK,
(PTR_TO_BTF_ID | PTR_TRUSTED).