-
Notifications
You must be signed in to change notification settings - Fork 473
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Linux: Add support for the mount namespace in kernels 6.8+ #1238
Linux: Add support for the mount namespace in kernels 6.8+ #1238
Conversation
Sorry for the last minute improvement. I realized that an extension object was more appropriate here, since it was already attached to the rb_root type via rb_node, rb_left, and rb_right. It is now ready for review |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good, thanks.
Reading #1187, does this PR require symbols generated with a patched |
@ikelos There's a bug in dwarf2json that prevents generating valid ISFs. Based on #63 and #57, it affects Linux kernel version 6.5+, which is pretty bad. Since this ticket addresses changes in the mount namespace for kernels 6.8+, it's indirectly affected. One of the comments suggests a workaround that’s currently awaiting review. I'm not sure if it's the best way to fix it, but it worked for me. Do you want to block this until they fix that bug in dwarf2json and make it dependent on the new version? |
Well, what I'm trying to avoid is users using an old symbol table, hitting whatever the bug is and then filing a new bug for a known problem. The question is how best to warn about the bad table. We can either do it on load of the bad table ("warning, you need to regenerate this using dwarf2json at least x.x.x") or whether we only do the check in known fields. I don't think our cache database keeps the metadata (which perhaps it should, since it'll have read all the symbols at least once) so that we could do full sweeps of their stores (the warnings may get annoying if we do that on every run and they don't care though?). The other options are to only flag on particular plugins that are affected, or to just do nothing and have vol failed on bad symbol tables without much notice. I'd prefer to avoid that last one unless it's a vanishingly small percentage change of affecting users (and we're willing to handle bugs people bring up about it). I think we've already got some code somewhere that checked for a symbol table's version, I think that was a per plugin check, lemme see if I can find it... Yeah, it's the
So we've got a |
Yeah, the dwarf2json issue hasn't been fixed yet, so there's no specific version to use for that check. There's no guarantee that it will be fixed in the next version, so using something like |
Resolved conflict following the recent merge of Linux Page Cache and IDR |
@ikelos volatilityfoundation/dwarf2json#66 and volatilityfoundation/dwarf2json#57 are done. Regarding our last comments, you want me to lock this to the dwarf2json 0.9.0 version? |
Yeah, just wondering if it needs to be across all plugins or just specific ones? I made PR #1305 without remembering about the function I already added, but I dunno whether it should be applied in a plug-in but phone basis (plus my PR breaks tests so I need to figure that out first...) |
The dwarf2json issue is not limited to this plugin, it could happen with any type that has a binding in Rust. If we want to add this check, it needs to be across all plugins. For instance: $ pahole ./vmlinux-6.8.0-41-generic -C sockaddr
struct sockaddr {
sa_family_t sa_family; /* 0 2 */
union {
char sa_data_min[14]; /* 2 14 */
struct {
struct {
} __empty_sa_data; /* 2 0 */
char sa_data[0]; /* 2 0 */
}; /* 2 0 */
}; /* 2 14 */
/* size: 16, cachelines: 1, members: 2 */
/* last cacheline: 16 bytes */
}; $ pahole ./vmlinux-6.8.0-41-generic --lang rust -C sockaddr
struct sockaddr {
u16 sa_family __attribute__((__aligned__(2))); /* 0 2 */
struct sockaddr__bindgen_ty_1 __bindgen_anon_1 __attribute__((__aligned__(1))); /* 2 14 */
/* size: 16, cachelines: 1, members: 2 */
/* forced alignments: 2 */
/* last cacheline: 16 bytes */
} __attribute__((__aligned__(2))); As you can see above, the Rust bindings don't have all members. Another example: $ pahole ./vmlinux-6.8.0-41-generic -C vm_area_struct | grep vm_start | wc -l
1
$ pahole ./vmlinux-6.8.0-41-generic --lang rust -C vm_area_struct | grep vm_start | wc -l
0 If |
This is dependent on #1305 then. Looks like we've got a pretty good workable solution, we just need to get it developed and merged in... |
I agree it could be better if #1305 goes first.. but technically it doesn't depend on that one. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good, sorry about the delay in reviewing.
In the context of #1187, this PR adds support for the mount namespace in kernels 6.8+. A basic Red-Black tree abstraction was also introduced to enable reuse in other parts of the framework if necessary.