Re: [PATCH net] gve: bound the device reported rule count in the flow rules query
From: Harshitha Ramamurthy
Date: Mon Nov 09 2026 - 14:24:25 EST
On Tue, Nov 3, 2026 at 10:13 PM Yehyeong Lee <yhlee@xxxxxxxxxxxxxxxxxx> wrote:
>
> gve_adminq_process_flow_rules_query() turns num_queried_rules, which the
> device wrote into the admin queue buffer, into a copy length:
>
> num_queried_rules = be32_to_cpu(descriptor->num_queried_rules);
> rule_info = (void *)(descriptor + 1);
> ...
> rule_info_len = num_queried_rules *
> sizeof(*flow_rules_cache->rules_cache);
> if (sizeof(*descriptor) + rule_info_len != total_memory_len) {
> dev_err(&priv->dev->dev, "flow rules query is out of memory.\n");
> return -ENOMEM;
> }
>
> memcpy(flow_rules_cache->rules_cache, rule_info, rule_info_len);
>
> Nothing bounds that length. The test above does not, because
> total_memory_len is descriptor->total_length and the same device wrote it,
> so it only asks the device to agree with itself.
>
> There are two consequences. For GVE_FLOW_RULE_QUERY_RULES the copy runs
> past the cache: at 47 rules it writes 4136 bytes into the 4048 byte
> rules_cache, and rules_cache_num is then set to that count for the ethtool
> paths to walk. For GVE_FLOW_RULE_QUERY_IDS the destination is large
> enough, but the descriptor and the rule array share one
> GVE_ADMINQ_BUFFER_SIZE block and the array starts after the descriptor, so
> counts of 1021 to 1024 read past that 4096 byte block. Those words reach
> userspace through ETHTOOL_GRXCLSRLALL, which needs no CAP_NET_ADMIN
> because net/ethtool/ioctl.c exempts it, and gve_get_flow_rule_ids() asks
> the device again on every call.
>
> Bound the count by the space the array actually has, the admin queue
> buffer less the descriptor, before the multiply that cannot then overflow
> either. That is 46 rules and 1020 ids, and it is never larger than the
> destination cache. The total_length test stays, since it still catches a
> descriptor whose payload was truncated.
>
> Reproduced with a KUnit harness that injects the device reply and is not
> part of this patch. Unpatched, 47 rules gave the report below, and 1021
gve, like other drivers, trusts device responses per
Documentation/process/threat-model.rst ('Non-conforming devices and
media') documented at
https://docs.kernel.org/process/threat-model.html.
The driver already communicates an 'available_length' field to the
device as part of gve_adminq_query_flow_rules() and no real device
would violate that. We don't harden against arbitrary devices. This
should be dropped.
> and 1024 ids copied 4084 and 4096 bytes from the descriptor's end, 4 and
> 16 bytes past the block. With the patch all three are rejected with
> -EINVAL, and 46 rules and 1020 ids still work.
>
> BUG: KASAN: slab-out-of-bounds in gve_adminq_process_flow_rules_query+0xe7/0x390
> Write of size 4136 at addr ffff8880027ae000 by task kunit_try_catch/33
>
> CPU: 0 UID: 0 PID: 33 Comm: kunit_try_catch Tainted: G N 7.3.0-dirty #8 PREEMPT(lazy)
> Tainted: [N]=TEST
> Call Trace:
> <TASK>
> dump_stack_lvl+0x3f/0x50
> print_report+0xcd/0x620
> ? gve_adminq_process_flow_rules_query+0xe7/0x390
> kasan_report+0xde/0x110
> ? gve_adminq_process_flow_rules_query+0xe7/0x390
> kasan_check_range+0x105/0x1b0
> __asan_memcpy+0x3c/0x60
> gve_adminq_process_flow_rules_query+0xe7/0x390
> gve_fr_rules_overflow_test+0xc9/0x430
> ? __pfx_gve_fr_rules_overflow_test+0x10/0x10
> ? __pfx___set_cpus_allowed_ptr+0x10/0x10
> ? __pfx_read_tsc+0x10/0x10
> ? ktime_get_ts64+0x192/0x250
> kunit_try_run_case+0x1ca/0x4b0
> ? __pfx_kunit_try_run_case+0x10/0x10
> ? kthread_affine_node+0x247/0x360
> ? __pfx_kthread_affine_node+0x10/0x10
> ? __pfx_kunit_try_run_case+0x10/0x10
> ? __pfx_kunit_generic_run_threadfn_adapter+0x10/0x10
> kunit_generic_run_threadfn_adapter+0x7b/0xe0
> kthread+0x36a/0x4f0
> ? __pfx_kthread+0x10/0x10
> ret_from_fork+0x34d/0x620
> ? __pfx_ret_from_fork+0x10/0x10
> ? __switch_to+0x22/0xde0
> ? __pfx_kthread+0x10/0x10
> ret_from_fork_asm+0x19/0x30
> </TASK>
>
> Allocated by task 33:
> kasan_save_stack+0x33/0x60
> kasan_save_track+0x14/0x30
> __kasan_kmalloc+0x8f/0xa0
> __kvmalloc_node_noprof+0x1e8/0x4e0
> gve_fr_ctx_init+0x24d/0x920
> gve_fr_rules_overflow_test+0xae/0x430
> kunit_try_run_case+0x1ca/0x4b0
> kunit_generic_run_threadfn_adapter+0x7b/0xe0
> kthread+0x36a/0x4f0
> ret_from_fork+0x34d/0x620
> ret_from_fork_asm+0x19/0x30
>
> The buggy address belongs to the object at ffff8880027ae000
> which belongs to the cache kmalloc-4k of size 4096
> The buggy address is located 0 bytes inside of
> allocated 4048-byte region [ffff8880027ae000, ffff8880027aefd0)
>
> The buggy address belongs to the physical page:
> page: refcount:0 mapcount:0 mapping:0000000000000000 index:0x0 pfn:0x27a8
> head: order:3 mapcount:0 entire_mapcount:0 nr_pages_mapped:0 pincount:0
> flags: 0x4000000000000040(head|zone=1)
> page_type: f5(slab)
> raw: 4000000000000040 ffff888001042140 dead000000000122 0000000000000000
> raw: 0000000000000000 0000000000040004 00000000f5000000 0000000000000000
> head: 4000000000000040 ffff888001042140 dead000000000122 0000000000000000
> head: 0000000000000000 0000000000040004 00000000f5000000 0000000000000000
> head: 4000000000000003 ffffea000009ea01 00000000ffffffff 00000000ffffffff
> head: 0000000000000000 0000000000000000 00000000ffffffff 0000000000000000
> page dumped because: kasan: bad access detected
>
> Memory state around the buggy address:
> ffff8880027aee80: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> ffff8880027aef00: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> >ffff8880027aef80: 00 00 00 00 00 00 00 00 00 00 fc fc fc fc fc fc
> ^
> ffff8880027af000: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
> ffff8880027af080: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
>
> Fixes: 57718b60df9b ("gve: Add flow steering adminq commands")
> Cc: stable@xxxxxxxxxxxxxxx
> Signed-off-by: Yehyeong Lee <yhlee@xxxxxxxxxxxxxxxxxx>
> ---
> drivers/net/ethernet/google/gve/gve_adminq.c | 10 ++++++++++
> 1 file changed, 10 insertions(+)
>
> diff --git a/drivers/net/ethernet/google/gve/gve_adminq.c b/drivers/net/ethernet/google/gve/gve_adminq.c
> index 8b215fca9c1b63..02c1f398f6e395 100644
> --- a/drivers/net/ethernet/google/gve/gve_adminq.c
> +++ b/drivers/net/ethernet/google/gve/gve_adminq.c
> @@ -1542,14 +1542,20 @@ static int gve_adminq_process_flow_rules_query(struct gve_priv *priv, u16 query_
> {
> struct gve_flow_rules_cache *flow_rules_cache = &priv->flow_rules_cache;
> u32 num_queried_rules, total_memory_len, rule_info_len;
> + u32 rule_info_max_len;
> void *rule_info;
>
> total_memory_len = be32_to_cpu(descriptor->total_length);
> num_queried_rules = be32_to_cpu(descriptor->num_queried_rules);
> rule_info = (void *)(descriptor + 1);
> + rule_info_max_len = GVE_ADMINQ_BUFFER_SIZE - sizeof(*descriptor);
>
> switch (query_opcode) {
> case GVE_FLOW_RULE_QUERY_RULES:
> + if (num_queried_rules >
> + rule_info_max_len / sizeof(*flow_rules_cache->rules_cache))
> + return -EINVAL;
> +
> rule_info_len = num_queried_rules * sizeof(*flow_rules_cache->rules_cache);
> if (sizeof(*descriptor) + rule_info_len != total_memory_len) {
> dev_err(&priv->dev->dev, "flow rules query is out of memory.\n");
> @@ -1560,6 +1566,10 @@ static int gve_adminq_process_flow_rules_query(struct gve_priv *priv, u16 query_
> flow_rules_cache->rules_cache_num = num_queried_rules;
> break;
> case GVE_FLOW_RULE_QUERY_IDS:
> + if (num_queried_rules >
> + rule_info_max_len / sizeof(*flow_rules_cache->rule_ids_cache))
> + return -EINVAL;
> +
> rule_info_len = num_queried_rules * sizeof(*flow_rules_cache->rule_ids_cache);
> if (sizeof(*descriptor) + rule_info_len != total_memory_len) {
> dev_err(&priv->dev->dev, "flow rule ids query is out of memory.\n");
>
> base-commit: 787268686c64be60023743b527645a907e38ccca
> --
> 2.43.0
>