Bug 496571 - False positive for null key passed to bpf_map_get_next_key syscall.
Summary: False positive for null key passed to bpf_map_get_next_key syscall.
Status: RESOLVED FIXED
Alias: None
Product: valgrind
Classification: Developer tools
Component: memcheck (other bugs)
Version First Reported In: 3.22.0
Platform: Ubuntu Linux
: NOR minor
Target Milestone: ---
Assignee: Paul Floyd
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2024-11-22 04:31 UTC by Ryan Mack
Modified: 2024-11-23 17:05 UTC (History)
1 user (show)

See Also:
Latest Commit:
Version Fixed/Implemented In:
Sentry Crash Report:


Attachments
Self contained reproduction case. (2.64 KB, text/x-csrc)
2024-11-22 06:31 UTC, Ryan Mack
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Ryan Mack 2024-11-22 04:31:39 UTC
The BPF function bpf_map_get_next_key takes a null key parameter to get the
first key in a map. Valgrind should not warn of an invalid pointer in this
case.

Locally tested fix:

diff --git a/coregrind/m_syswrap/syswrap-linux.c b/coregrind/m_syswrap/syswrap-linux.c
index 177712117..9be77992c 100644
--- a/coregrind/m_syswrap/syswrap-linux.c
+++ b/coregrind/m_syswrap/syswrap-linux.c
@@ -12993,7 +12993,10 @@ PRE(sys_bpf)
             }
             /* Get size of key for this map. */
             if (bpf_map_get_sizes(attr->map_fd, &key_size, &value_size)) {
-               PRE_MEM_READ("bpf(attr->key)", attr->key, key_size);
+               /* Key is null when getting first entry in map. */
+               if (attr->key) {
+                  PRE_MEM_READ("bpf(attr->key)", attr->key, key_size);
+               }
                PRE_MEM_WRITE("bpf(attr->next_key)", attr->next_key, key_size);
             }
          }
Comment 1 Paul Floyd 2024-11-22 05:08:27 UTC
Thanks. Bpf needs some attention.

Do you have a small reproducer for this case?
Comment 2 Ryan Mack 2024-11-22 06:31:37 UTC
Created attachment 176031 [details]
Self contained reproduction case.
Comment 3 Paul Floyd 2024-11-22 06:48:54 UTC
Thanks for the quick reply.

I'll test the patch and testcase this weekend.
Comment 4 Paul Floyd 2024-11-23 17:05:55 UTC
Thanks for the patch!

commit 75ca7437c97a703b7a729d8694743ddde3762713 (HEAD -> master, origin/master, origin/HEAD)
Author: Ryan Mack <rmack@uptycs.com>
Date:   Sat Nov 23 18:02:21 2024 +0100

    Bug 496571 - False positive for null key passed to bpf_map_get_next_key syscall.
    
    No regtest added because BPF requires privileges. See the bugzilla item
    for example usage.