-
Notifications
You must be signed in to change notification settings - Fork 104
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
[BUG] lsof tests fail with kernel 6.9 #317
Comments
This works around the issue:
--- a/lib/dialects/linux/tests/case-20-pidfd-pid.bash
+++ b/lib/dialects/linux/tests/case-20-pidfd-pid.bash
@@ -11,7 +11,8 @@ $TARGET | (
exit 77
fi
line=$($lsof -p $pid -a -d $fd -F pfn| tr '\n' ' ')
- if ! fgrep -q "p${pid} f${fd} n[pidfd:$pid]" <<<"$line"; then
+ if ! fgrep -q "p${pid} f${fd} n[pidfd:$pid]" <<<"$line" &&
+ ! fgrep -q "p${pid} f${fd} npidfd" <<<"$line"; then
$lsof -p $pid -a -d $fd -F pfn
echo
echo $line |
Thanks, please submit a pull request. I will test on different kernel versions. |
pidfd was changed by commit cb12fd8e0dab (pidfd: add pidfs). lsof now returns pidfd and not [pidfd:$pid]. Adapt the test to that (and accept both forms). Fixes lsof-org#317.
pidfd was changed by commit cb12fd8e0dab (pidfd: add pidfs). lsof now returns pidfd and not [pidfd:$pid]. Adapt the test to that (and accept both forms). Fixes lsof-org#317.
pidfd was changed by commit cb12fd8e0dab (pidfd: add pidfs). lsof now returns pidfd and not [pidfd:$pid]. Adapt the test to that (and accept both forms). Fixes lsof-org#317.
Ah, I see where is the problem: Before:
After:
|
Likely, see also: |
I am not sure what's the best solution here:
|
It does:
But BTW util-linux had to adapt too, not sure if lsof needs to: util-linux/util-linux#2866 |
According the code, it checks if it is a regular file before detecting anon_inode, however in Linux 6.9 it is a regular file, so the logic for pidfd is not executed. |
I have implemented the second option
I have implemented the second option in #319 |
This is not enough, the complete fix: --- a/fs/libfs.c
+++ b/fs/libfs.c
@@ -2026,7 +2026,6 @@ static struct dentry *prepare_anon_dentry(struct dentry **stashed,
}
/* Notice when this is changed. */
- WARN_ON_ONCE(!S_ISREG(inode->i_mode));
WARN_ON_ONCE(!IS_IMMUTABLE(inode));
dentry = d_alloc_anon(sb);
diff --git a/fs/pidfs.c b/fs/pidfs.c
index a63d5d24..f51a794f 100644
--- a/fs/pidfs.c
+++ b/fs/pidfs.c
@@ -201,10 +201,7 @@ static const struct super_operations pidfs_sops = {
static char *pidfs_dname(struct dentry *dentry, char *buffer, int buflen)
{
- struct inode *inode = d_inode(dentry);
- struct pid *pid = inode->i_private;
-
- return dynamic_dname(buffer, buflen, "pidfd:[%llu]", pid->ino);
+ return dynamic_dname(buffer, buflen, "anon_inode:[pidfd]");
}
static const struct dentry_operations pidfs_dentry_operations = {
@@ -217,6 +214,7 @@ static int pidfs_init_inode(struct inode *inode, void *data)
{
inode->i_private = data;
inode->i_flags |= S_PRIVATE;
+ inode->i_mode &= ~S_IFREG;
inode->i_mode |= S_IRWXU;
inode->i_op = &pidfs_inode_operations;
inode->i_fop = &pidfs_file_operations; Let's see how they fix it in the end. |
pidfs started using much saner inodes in commit b28ddcc ("pidfs: convert to path_from_stashed() helper"), but that exposed the fact that lsof had some knowledge of just how odd our old anon_inode usage was. For example, legacy anon_inodes hadn't even initialized the inode type in the inode mode, so everything had a type of zero. So sane tools like 'stat' would report these files as "weird file", but 'lsof' instead used that (together with the name of the link in proc) to notice that it's an anonymous inode, and used it to detect pidfd files. Let's keep our internal new sane inode model, but mask the file type bits at 'stat()' time in the getattr() function we already have, and by making the dentry name match what lsof expects too. This keeps our internal models sane, but should make user space see the same old odd behavior. Reported-by: Jiri Slaby <jirislaby@kernel.org> Link: https://lore.kernel.org/all/a15b1050-4b52-4740-a122-a4d055c17f11@kernel.org/ Link: lsof-org/lsof#317 Cc: Alexander Viro <viro@zeniv.linux.org.uk> Cc: Seth Forshee <sforshee@kernel.org> Cc: Tycho Andersen <tycho@tycho.pizza> Signed-off-by: Christian Brauner <brauner@kernel.org> Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
For now we're mimicking the old pidfd behavior but we will try to flip the switch to the new format:
where So I really appreciate the change to accomodate pidfs. Fwiw, both |
Thanks for your hard work on Linux kernel. I am okay with all formats (the legacy one, the current one and maybe a future one), and happy to accommodate them in lsof. |
As described in
broke lsof tests:
Without the commit, lsof shows:
With the commit:
I am not sure whether this is a kernel or lsof fault...
The text was updated successfully, but these errors were encountered: