| Message ID | 20220628161948.475097-1-kpsingh@kernel.org (mailing list archive) |
|---|---|
| Headers | show |
| Series | Add bpf_getxattr | expand |
On Tue, Jun 28, 2022 at 04:19:43PM +0000, KP Singh wrote: > 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? :) > > > > Benjamin Tissoires (1): > btf: Add a new kfunc set which allows to mark a function to be > sleepable > > KP Singh (4): > bpf: kfunc support for ARG_PTR_TO_CONST_STR > bpf: Allow kfuncs to be used in LSM programs > bpf: Add a bpf_getxattr kfunc > bpf/selftests: Add a selftest for bpf_getxattr > > include/linux/bpf_verifier.h | 2 + > include/linux/btf.h | 2 + > kernel/bpf/btf.c | 43 ++++++++- > kernel/bpf/verifier.c | 89 +++++++++++-------- > kernel/trace/bpf_trace.c | 42 +++++++++ > .../testing/selftests/bpf/prog_tests/xattr.c | 54 +++++++++++ > tools/testing/selftests/bpf/progs/xattr.c | 37 ++++++++ > 7 files changed, 229 insertions(+), 40 deletions(-) > create mode 100644 tools/testing/selftests/bpf/prog_tests/xattr.c > create mode 100644 tools/testing/selftests/bpf/progs/xattr.c > > -- > 2.37.0.rc0.161.g10f37bed90-goog >
On Tue, Jun 28, 2022 at 7:13 PM Christian Brauner <brauner@kernel.org> wrote: > > On Tue, Jun 28, 2022 at 04:19:43PM +0000, KP Singh wrote: > > 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\ See: https://lore.kernel.org/bpf/20220624045636.3668195-1-kpsingh@kernel.org/ > > > > > > > > > Benjamin Tissoires (1): > > btf: Add a new kfunc set which allows to mark a function to be > > sleepable > > > > KP Singh (4): > > bpf: kfunc support for ARG_PTR_TO_CONST_STR > > bpf: Allow kfuncs to be used in LSM programs > > bpf: Add a bpf_getxattr kfunc > > bpf/selftests: Add a selftest for bpf_getxattr > > > > include/linux/bpf_verifier.h | 2 + > > include/linux/btf.h | 2 + > > kernel/bpf/btf.c | 43 ++++++++- > > kernel/bpf/verifier.c | 89 +++++++++++-------- > > kernel/trace/bpf_trace.c | 42 +++++++++ > > .../testing/selftests/bpf/prog_tests/xattr.c | 54 +++++++++++ > > tools/testing/selftests/bpf/progs/xattr.c | 37 ++++++++ > > 7 files changed, 229 insertions(+), 40 deletions(-) > > create mode 100644 tools/testing/selftests/bpf/prog_tests/xattr.c > > create mode 100644 tools/testing/selftests/bpf/progs/xattr.c > > > > -- > > 2.37.0.rc0.161.g10f37bed90-goog > >
On Tue, Jun 28, 2022 at 7:20 PM KP Singh <kpsingh@kernel.org> wrote: > > On Tue, Jun 28, 2022 at 7:13 PM Christian Brauner <brauner@kernel.org> wrote: > > > > On Tue, Jun 28, 2022 at 04:19:43PM +0000, KP Singh wrote: > > > 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 > > See: https://lore.kernel.org/bpf/20220624045636.3668195-1-kpsingh@kernel.org/ > > > > > > > > > > > > > > > > Benjamin Tissoires (1): > > > btf: Add a new kfunc set which allows to mark a function to be > > > sleepable > > > > > > KP Singh (4): > > > bpf: kfunc support for ARG_PTR_TO_CONST_STR > > > bpf: Allow kfuncs to be used in LSM programs > > > bpf: Add a bpf_getxattr kfunc > > > bpf/selftests: Add a selftest for bpf_getxattr > > > > > > include/linux/bpf_verifier.h | 2 + > > > include/linux/btf.h | 2 + > > > kernel/bpf/btf.c | 43 ++++++++- > > > kernel/bpf/verifier.c | 89 +++++++++++-------- > > > kernel/trace/bpf_trace.c | 42 +++++++++ > > > .../testing/selftests/bpf/prog_tests/xattr.c | 54 +++++++++++ > > > tools/testing/selftests/bpf/progs/xattr.c | 37 ++++++++ > > > 7 files changed, 229 insertions(+), 40 deletions(-) > > > create mode 100644 tools/testing/selftests/bpf/prog_tests/xattr.c > > > create mode 100644 tools/testing/selftests/bpf/progs/xattr.c > > > > > > -- > > > 2.37.0.rc0.161.g10f37bed90-goog > > >
On Tue, Jun 28, 2022 at 07:21:42PM +0200, KP Singh wrote: > On Tue, Jun 28, 2022 at 7:20 PM KP Singh <kpsingh@kernel.org> wrote: > > On Tue, Jun 28, 2022 at 7:13 PM Christian Brauner <brauner@kernel.org> wrote: > > > On Tue, Jun 28, 2022 at 04:19:43PM +0000, KP Singh wrote: > > > > 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. 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 xattr namespace(s) this hook should belong/be constrained to, whether you're going to ask for a setxattr hook next, etc. At minimum this is going to need a bunch of documentation for people to understand how to use this - where can I find that? Cheers, Dave.
On Wed, Jun 29, 2022 at 3:36 AM Dave Chinner <david@fromorbit.com> wrote: > > On Tue, Jun 28, 2022 at 07:21:42PM +0200, KP Singh wrote: > > On Tue, Jun 28, 2022 at 7:20 PM KP Singh <kpsingh@kernel.org> wrote: > > > On Tue, Jun 28, 2022 at 7:13 PM Christian Brauner <brauner@kernel.org> wrote: > > > > On Tue, Jun 28, 2022 at 04:19:43PM +0000, KP Singh wrote: > > > > > 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/ 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
On Wed, Jun 29, 2022 at 4:00 AM KP Singh <kpsingh@kernel.org> wrote: > > On Wed, Jun 29, 2022 at 3:36 AM Dave Chinner <david@fromorbit.com> wrote: > > > > On Tue, Jun 28, 2022 at 07:21:42PM +0200, KP Singh wrote: > > > On Tue, Jun 28, 2022 at 7:20 PM KP Singh <kpsingh@kernel.org> wrote: > > > > On Tue, Jun 28, 2022 at 7:13 PM Christian Brauner <brauner@kernel.org> wrote: > > > > > On Tue, Jun 28, 2022 at 04:19:43PM +0000, KP Singh wrote: > > > > > > 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/ > > 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. In any case, I will update the cover letter with some use-cases we ideated over in LSF/MM/BPF in the next version. - KP > > Cheers, > - KP > > > > > Cheers, > > > > Dave. > > -- > > Dave Chinner > > david@fromorbit.com
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 Benjamin Tissoires (1): btf: Add a new kfunc set which allows to mark a function to be sleepable KP Singh (4): bpf: kfunc support for ARG_PTR_TO_CONST_STR bpf: Allow kfuncs to be used in LSM programs bpf: Add a bpf_getxattr kfunc bpf/selftests: Add a selftest for bpf_getxattr include/linux/bpf_verifier.h | 2 + include/linux/btf.h | 2 + kernel/bpf/btf.c | 43 ++++++++- kernel/bpf/verifier.c | 89 +++++++++++-------- kernel/trace/bpf_trace.c | 42 +++++++++ .../testing/selftests/bpf/prog_tests/xattr.c | 54 +++++++++++ tools/testing/selftests/bpf/progs/xattr.c | 37 ++++++++ 7 files changed, 229 insertions(+), 40 deletions(-) create mode 100644 tools/testing/selftests/bpf/prog_tests/xattr.c create mode 100644 tools/testing/selftests/bpf/progs/xattr.c