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 kfuncsHm, 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.pdfThere'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