#include <stdio.h> #include <stdint.h> #include <fcntl.h> #include <sys/ioctl.h> #include <linux/fs.h> int main(int argc, char** argv) { char name[256]; struct procmap_query pq = { .size = sizeof(pq), .query_addr = (uintptr_t)main, .vma_name_size = 256, .vma_name_addr = (uintptr_t)name }; int fd = open("/proc/self/maps", O_RDONLY); ioctl(fd, PROCMAP_QUERY, &pq); puts(name); } Install Linux >= 6.11, then compile and run the above. No particular flags needed, gcc's and valgrind's defaults are fine. Expected: Print the usual Valgrind header/footer, and the path of the executable. Actual: Also prints various warnings about the name variable being uninitialized. Discovered on Debian stable (3.24), also reproduces on 3.25.1. Probably needs something similar to bug 333817.
Created attachment 186956 [details] WIP hotfix that avoids the false positive.
Created attachment 186990 [details] proposed patch Also checked in to try-branch so that it can get buildbot-tested: https://sourceware.org/git/?p=valgrind.git;a=shortlog;h=refs/heads/users/mcermak/try-bug508328
I can't determine how (or if) the first patch works, but the second one looks good to my (admittedly inexperienced in this project) eyes. Except the kernel will use the build_id_{size,addr} fields the same way as vma_name_{size,addr}. You should duplicate that.
Created attachment 187024 [details] additional patch addressing Comment#3 Agreed. Follow-up patch addressing Comment#3 attached. Try-branch updated. The previous patch passed testing [1]. Buildbots should pick up this update soon.. [1] https://builder.sourceware.org/buildbot/#/builders/243/builds/109
The new testcase needs to be moved under memcheck/tests/linux since it is linux kernel specific. You will want to check ML_(safe_to_deref) (pq, sizeof(struct vki_procmap_query) in the PRE handler before doing any checks that need dereferencing pq. Also check the other "in" fields (size, query_flags and query_addr) with PRE_FIELD_READ in the PRE handler. Also mark the other "out" fields (vma_*, inode, dev_*) as written with POST_FIELD_WRITE in the POST handler. (In the POST handler you can probably just assume everything is written/defined with POST_MEM_WRITE(pq, pq-<size) instead of doing individual POST_FIELD_WRITEs)
Also looking at the try builds (thanks for doing those!) it looks like for the testcase you also need a configure check to make sure PROCMAP_QUERY is defined in linux/fs.h. https://builder.sourceware.org/buildbot/#/changes/102828 shows various systems with an older kernel that doesn't define it.
Thank you for your review! I've addressed your comments in my branch users/mcermak/try-bug508328: $ git log --pretty --oneline -4 6605bba9b (HEAD -> users/mcermak/try-bug508328, origin/users/mcermak/try-bug508328) PR508328: Run ioctl_procmap_query.vgtest conditionally 0a779b583 PR508328: Reflect the testcase move in .gitignore b99a16afb PR508328: Check all the members of struct procmap_query 257ba83ca PR508328: Move ioctl_procmap_query.vgtest to the linux subdirectory $ https://sourceware.org/git/?p=valgrind.git;a=shortlog;h=refs/heads/users/mcermak/try-bug508328
Buildbot test results green: https://builder.sourceware.org/buildbot/#/builders/243/builds/111 .
Could you post that as a squash patch? I think it looks good. I would only remove the SET_STATUS_Failure(VKI_EFAULT) after the safe_to_deref check. The kernel will return that anyway, but it might accept a larger/smaller struct in the future?
Created attachment 187159 [details] updated patch Attaching squashed patch addressing the previous comment.
(In reply to mcermak from comment #10) > Created attachment 187159 [details] > updated patch > > Attaching squashed patch addressing the previous comment. Looks good. Since it might be that the testcase isn't build (if the configure check fails) I think you should have something like: prereq: test -x ioctl_procmap_query in ioctl_procmap_query.vgtest otherwise you get failures on old kernels, where the test should be skipped. Please push with that change.
Makes sense, thanks a ton for the review. Pushed as commit 2b4b31de6965c3af5ecaca2c33b039eb72ec3706 .