On 11/4/20 3:29 AM, Daniel Xu wrote:
do_strncpy_from_user() may copy some extra bytes after the NUL terminator into the destination buffer. This usually does not matter for normal string operations. However, when BPF programs key BPF maps with strings, this matters a lot. A BPF program may read strings from user memory by calling the bpf_probe_read_user_str() helper which eventually calls do_strncpy_from_user(). The program can then key a map with the resulting string. BPF map keys are fixed-width and string-agnostic, meaning that map keys are treated as a set of bytes. The issue is when do_strncpy_from_user() overcopies bytes after the NUL terminator, it can result in seemingly identical strings occupying multiple slots in a BPF map. This behavior is subtle and totally unexpected by the user. This commit uses the proper word-at-a-time APIs to avoid overcopying. Signed-off-by: Daniel Xu <dxu@xxxxxxxxx>
It looks like this is a regression from the recent refactoring of the mem probing util functions? Could we add a Fixes tag and then we'd also need to target the fix against bpf tree instead of bpf-next, no? Moreover, a BPF kselftest would help to make sure it doesn't regress in future again. Thanks, Daniel