aya: fix proc maps parsing for deleted files #1424
Conversation
✅ Deploy Preview for aya-rs-docs ready!Built without sensitive environment variables
To edit notification comments on pull requests, go to your Netlify project configuration. |
tamird
left a comment
tamird
left a comment
There was a problem hiding this comment.
Hi. This needs a new test.
Please keep the logic is strict (ignoring precisely what is expected).
Satar07
force-pushed
the
fix/proc-maps-parsing
branch
3 times, most recently
from
46c3127 to
4ec4657
Compare
currently, `procmapentry::parse` enforces a strict format where the path must be the last token in the line. this causes parsing failures when mapped files have been deleted, as the kernel appends a " (deleted)" suffix (e.g. "/usr/lib/libfoo.so (deleted)"). this strictness causes `find_library_path_by_name` to fail entirely if such lines exist, preventing uprobes from attaching. this patch relaxes the parsing logic by accepting the first token of the path field as the valid path if it starts with '/', ignoring any subsequent tokens like the deleted marker. Signed-off-by: cyril <multya77@gmail.com>
Satar07
force-pushed
the
fix/proc-maps-parsing
branch
from
4ec4657 to
2bf5bb2
Compare
|
Thanks! I've updated the logic to strictly accept only the (deleted) suffix after an absolute path. Any other trailing tokens will still cause a parse error. I also added a test case to cover this. |
|
As mentioned in an earlier comment that was perhaps missed: can we do this with less rewriting in the LLM's preferred style? |
|
Apologies for the noise. To be honest, I ran into this issue while working on an Android eBPF tool and used an LLM to help quickly prototype a fix. I was eager to contribute the solution back upstream but realized I shouldn't have pushed the generated code without refining it first. I genuinely appreciate the rigorous review process here—it clearly shows why Aya is such a high-quality library. I'll rework the implementation to address this. |
tamird
force-pushed
the
fix/proc-maps-parsing
branch
from
c60a3b4 to
c540831
Compare
|
@Satar07 I pushed another commit to reduce the diff further. Does this look reasonable to you? |
tamird
force-pushed
the
fix/proc-maps-parsing
branch
from
c540831 to
5e7f49d
Compare
|
That works for me! I appreciate you jumping in to clean this up. |
3 participants
Motivation
Currently,
ProcMapEntry::parseenforces a strict format where the path must be the last token in the line.This causes parsing failures when mapped libraries have been updated or deleted, as the kernel appends a
(deleted)suffix to the path in/proc/pid/maps(e.g.,/usr/lib/libc.so (deleted)).Because
split_whitespacetreats the suffix as a separate token, the current logic returns an error for such lines. This causesfind_library_path_by_nameto fail entirely, preventing uprobes from attaching even if the target library is valid.Solution
This PR relaxes the parsing logic in
ProcMapEntry::parsewhile maintaining backward compatibility with existing tests:/, it is accepted as the path. Any subsequent tokens (specifically the(deleted)suffix) are ignored.Testing
cargo testpasses (including strict parsing tests).Fixes: #1423
This change is