Thread (36 messages) 36 messages, 7 authors, 2022-07-01

Re: [PATCH v5 bpf-next 0/5] Add bpf_getxattr

From: KP Singh <kpsingh@kernel.org>
Date: 2022-06-29 02:00:36
Also in: bpf, linux-fsdevel

On Wed, Jun 29, 2022 at 3:36 AM Dave Chinner [off-list ref] wrote:
On Tue, Jun 28, 2022 at 07:21:42PM +0200, KP Singh wrote:
quoted
On Tue, Jun 28, 2022 at 7:20 PM KP Singh [off-list ref] wrote:
quoted
On Tue, Jun 28, 2022 at 7:13 PM Christian Brauner [off-list ref] wrote:
quoted
On Tue, Jun 28, 2022 at 04:19:43PM +0000, KP Singh wrote:
quoted
v4 -> v5

- Fixes suggested by Andrii

v3 -> v4

- Fixed issue incorrect increment of arg counter
- Removed __weak and noinline from kfunc definiton
- Some other minor fixes.

v2 -> v3

- Fixed missing prototype error
- Fixes suggested by other Joanne and Kumar.

v1 -> v2

- Used kfuncs as suggested by Alexei
- Used Benjamin Tissoires' patch from the HID v4 series to add a
  sleepable kfunc set (I sent the patch as a part of this series as it
  seems to have been dropped from v5) and acked it. Hope this is okay.
- Added support for verifying string constants to kfuncs
Hm, I mean this isn't really giving any explanation as to why you are
doing this. There's literally not a single sentence about the rationale?
Did you accidently forget to put that into the cover letter? :)

Yes, actually I did forget to copy paste :)

Foundation for building more complex security policies using the
BPF LSM as presented in LSF/MM/BPF:

http://vger.kernel.org/bpfconf2022_material/lsfmmbpf2022-xattr.pdf\
And my copy paste skills are getting worse (with the back-slash removed):

http://vger.kernel.org/bpfconf2022_material/lsfmmbpf2022-xattr.pdf
There's literally zero information in that link, so I still have no
clue on what this does and how it interacts with filesystem xattr
code.
This is literally a wrapper around __vfs_getxattr which is an exported
symbol. So, the interaction with the xattr code is the same as
__vfs_getxattr interacts currently.

ssize_t bpf_getxattr(struct dentry *dentry, struct inode *inode,
const char *name, void *value, int value__sz)
{
return __vfs_getxattr(dentry, inode, name, value, value__sz);
}

The reason for the wrapper is that the BPF verifier offers
extra checks on the arguments passed.

https://lore.kernel.org/bpf/20210325015240.1550074-1-kafai@fb.com/T/ (local)

has more information on the kfunc support.
So for those of us who have zero clue as to what you are trying to
do, please write a cover letter containing a non-zero amount of
information.  i.e.  a description of the problem, the threat model
being addressed, the design of the infrastructure that needs this
hook, document assumptions that have been made (e.g. for
accessing inode metadata atomically from random bpf contexts), what
The intention is to use this in BPF programs which can only be loaded
with CAP_SYS_ADMIN.
We are currently planning on limiting the usage of this kfunc
to the sleepable LSM hooks listed here:

https://git.kernel.org/pub/scm/linux/kernel/git/bpf/bpf-next.git/tree/kernel/bpf/bpf_lsm.c#n169
xattr namespace(s) this hook should belong/be constrained to,
whether you're going to ask for a setxattr hook next, etc.
Fair point, I will resend the series with the details.
At minimum this is going to need a bunch of documentation for people
to understand how to use this - where can I find that?
There are a bunch of examples in selftests on how to use kfuncs in BPF
and we added a selftests (there is a simple selftests added with this patch
too).

As to how we will use xattrs to create security policies or use this
functionality for
logging, this is work in progress.

Cheers,
- KP
Cheers,

Dave.
--
Dave Chinner
david@fromorbit.com
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help