On Fri, 17 May 2024 at 00:54, Jiri Slaby <jirislaby@xxxxxxxxxx> wrote:
>
> inode->i_private = data;
> inode->i_flags |= S_PRIVATE;
> + inode->i_mode &= ~S_IFREG;
That is not a sensible operation. S_IFREG isn't a bit mask.
But it looks like 'anon_inode' traditionally had *no* type bytes at
all. That's literally crazy.
Doing a 'stat -L' on one in /proc/X/fd/Y will correctly say "weird
file" about them.
What a crock. That's horrible, and we apparently never noticed how
broken anon_inodes were because nobody really cared. But then lsof
seems to have done the *opposite* and just said (for unfathomable
reasons) "this can't be a normal regular file".
But I can't actually find that code in lsof. I see
if (rest && rest[0] == '[' && rest[1] == 'p')
fdinfo_mask |= FDINFO_PID;
which only checks that the name starts with '[p'. Hmm.
[ Time passes, I go looking ]
Oh Christ. It's process_proc_node:
type = s->st_mode & S_IFMT;
switch (type) {
...
case 0:
if (!strcmp(p, "anon_inode"))
Lf->ntype = Ntype = N_ANON_INODE;
break;
so yes, process_proc_node() really seems to have intentionally noticed
that our anon inodes forgot to put a file type in the st_mode, and
together with the path from readlink matching 'anon_inode' is how lsof
determines it's one of the special inodes.
So yeah, we made a mistake, and then lsof decided that mistake was a feature.
But that does mean that we probably just have to live in the bed we made.
But that
> + inode->i_mode &= ~S_IFREG;
is still very very wrong. It should use the proper bit mask: S_IFMT.
And we'd have to add a big comment about our historical stupidity that
we are perpetuating.
Oh well.
Linus
