filesystem: Add labeling for pidfs.

[pebenito]

Add task SID labeling for pidfs, which is the new backing pseudo filesystem for pidfds.

The existing rules will allow domains to open pidfds and use them internally, but other domains will require additional access (fd and file) when passing around the pidfd.

[pebenito]

Need to decide if we should use task sid or a single common genfs label, like anon_inode.

@stephensmalley @jwcart2 @pcmoore if you have thoughts

[brauner]

This will also allow you to do fine-grained labeling finally because it now goes through the regular security_file_open().

[cgzones]

This requires https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/commit/fs/pidfs.c?h=next-20240223&id=a1a466d5af6c8ed03bfa30305b89cfe8bce1f3f9 (so linux-next for now)

[pcmoore]

As a pidfd is a reference to a task, I believe it should use the task's SID/label.

[pebenito]

There is an issue where if process A does pidfd_open() on process B, the pidfs entries will have process A's context instead of process B's, which is undesirable. Not sure if @brauner 's change above addresses it but want to note it here.

cc @pcmoore

[pcmoore]

There is an issue where if process A does pidfd_open() on process B, the pidfs entries will have process A's context instead of process B's, which is undesirable ...

"undesirable" is certainly a mild way to put it ;) It's definitely wrong and something we need to resolve. I'll take a look it as I suspect this is going to require kernel code to handle properly.

[brauner]

he pidfs entries will have process A's context instead of process B's

I don't understand what that is supposed to mean? The pidfd entries are anonymous inodes. They don't have any process credentials attached to them?

I was talking about file->f_cred, e.g., if you do pidfd_open(1234, ...) then it's the same as open("/proc/1234", ...). In both cases the opened file file->f_cred will be set to the opener's credentials. That's literally how file->f_cred is defined; to be able to go back to the opener's credentials.

[brauner]

Open File Credentials
=====================

When a new file is opened, a reference is obtained on the opening task's
credentials and this is attached to the file struct as ``f_cred`` in place of
``f_uid`` and ``f_gid``.  Code that used to access ``file->f_uid`` and
``file->f_gid`` should now access ``file->f_cred->fsuid`` and
``file->f_cred->fsgid``.

Documentation/filesystems/credentials.rst

[pebenito]

I don't understand what that is supposed to mean? The pidfd entries are anonymous inodes. They don't have any process credentials attached to them?

It might be my lack of knowledge injecting confusion into this. I'll leave it to Paul and the other kernel devs to clarify.

[pcmoore]

I haven't had a chance to look at any code yet, I'm basing my comments purely on what has been written here and the pidfs patchset cover letter, but you've explicitly mentioned moving away from the anonymous inode infrastructure @brauner:

This moves pidfds from the anonymous inode infrastructure to a tiny pseudo filesystem.

... if this isn't the case, that would be good to know.

[brauner]

I haven't had a chance to look at any code yet, I'm basing my comments purely on what has been written here and the pidfs patchset cover letter, but you've explicitly mentioned moving away from the anonymous inode infrastructure @brauner:

This moves pidfds from the anonymous inode infrastructure to a tiny pseudo filesystem.

... if this isn't the case, that would be good to know.

Anonymous inodes are just inodes marked with S_PRIVATE, so no we're not moving away from that.

[brauner]

We're just using our own superblock just like dma, drm, cxl, or oxcl, aio, iomem, or secretmem.

[pcmoore]

I haven't had a chance to look at any code yet, I'm basing my comments purely on what has been written here and the pidfs patchset cover letter, but you've explicitly mentioned moving away from the anonymous inode infrastructure @brauner:

This moves pidfds from the anonymous inode infrastructure to a tiny pseudo filesystem.

... if this isn't the case, that would be good to know.

Anonymous inodes are just inodes marked with S_PRIVATE, so no we're not moving away from that.

Sure, but you did mention that you were moving away from the anonymous inode infrastructure which could be relevant from a LSM perspective. I still need to check the code.

[pcmoore]

There is also still the original issue that we likely want to label pidfds based on their associated task, similar to what we do with process entries in procfs. While the concept is independent of the pidfd/pidfs implementation, how we satisfy this requirement is obviously very implementation specific.

[brauner]

Sure https://git.kernel.org/pub/scm/linux/kernel/git/vfs/vfs.git vfs.pidfd

[github-actions[bot]]

This PR has not had any recent activity. It will be closed in 7 days if it makes no further progress.

[github-actions[bot]]

This PR has not had any recent activity. It will be closed in 7 days if it makes no further progress.

[github-actions[bot]]

Closing stale PR.