| Message ID | 3a2903574a4d03f73230047866112b2dad9b4a9e.1622467740.git.rgb@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [1/2] audit: add filtering for io_uring records, addendum | expand |
Hi Richard,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on pcmoore-audit/next]
[also build test ERROR on v5.13-rc4 next-20210528]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url: https://github.com/0day-ci/linux/commits/Richard-Guy-Briggs/audit-add-filtering-for-io_uring-records-addendum/20210531-214941
base: https://git.kernel.org/pub/scm/linux/kernel/git/pcmoore/audit.git next
config: parisc-randconfig-r015-20210531 (attached as .config)
compiler: hppa-linux-gcc (GCC) 9.3.0
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://github.com/0day-ci/linux/commit/07a3e22a2f984838bc98b43b58e8ef08e9353483
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Richard-Guy-Briggs/audit-add-filtering-for-io_uring-records-addendum/20210531-214941
git checkout 07a3e22a2f984838bc98b43b58e8ef08e9353483
# save the attached .config to linux build tree
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross ARCH=parisc
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
All errors (new ones prefixed by >>):
kernel/auditfilter.c: In function 'audit_to_inode':
>> kernel/auditfilter.c:155:24: error: 'AUDIT_FILTER_URING_EXIT' undeclared (first use in this function); did you mean 'AUDIT_FILTER_EXIT'?
155 | krule->listnr != AUDIT_FILTER_URING_EXIT) ||
| ^~~~~~~~~~~~~~~~~~~~~~~
| AUDIT_FILTER_EXIT
kernel/auditfilter.c:155:24: note: each undeclared identifier is reported only once for each function it appears in
kernel/auditfilter.c: In function 'audit_to_entry_common':
kernel/auditfilter.c:252:7: error: 'AUDIT_FILTER_URING_EXIT' undeclared (first use in this function); did you mean 'AUDIT_FILTER_EXIT'?
252 | case AUDIT_FILTER_URING_EXIT:
| ^~~~~~~~~~~~~~~~~~~~~~~
| AUDIT_FILTER_EXIT
kernel/auditfilter.c: In function 'audit_add_rule':
kernel/auditfilter.c:986:28: error: 'AUDIT_FILTER_URING_EXIT' undeclared (first use in this function); did you mean 'AUDIT_FILTER_EXIT'?
986 | entry->rule.listnr == AUDIT_FILTER_URING_EXIT) {
| ^~~~~~~~~~~~~~~~~~~~~~~
| AUDIT_FILTER_EXIT
--
kernel/audit_watch.c: In function 'audit_to_watch':
>> kernel/audit_watch.c:187:24: error: 'AUDIT_FILTER_URING_EXIT' undeclared (first use in this function); did you mean 'AUDIT_FILTER_EXIT'?
187 | krule->listnr != AUDIT_FILTER_URING_EXIT) ||
| ^~~~~~~~~~~~~~~~~~~~~~~
| AUDIT_FILTER_EXIT
kernel/audit_watch.c:187:24: note: each undeclared identifier is reported only once for each function it appears in
--
kernel/audit_tree.c: In function 'audit_make_tree':
>> kernel/audit_tree.c:731:23: error: 'AUDIT_FILTER_URING_EXIT' undeclared (first use in this function); did you mean 'AUDIT_FILTER_EXIT'?
731 | rule->listnr != AUDIT_FILTER_URING_EXIT) ||
| ^~~~~~~~~~~~~~~~~~~~~~~
| AUDIT_FILTER_EXIT
kernel/audit_tree.c:731:23: note: each undeclared identifier is reported only once for each function it appears in
vim +155 kernel/auditfilter.c
149
150 /* Translate an inode field to kernel representation. */
151 static inline int audit_to_inode(struct audit_krule *krule,
152 struct audit_field *f)
153 {
154 if ((krule->listnr != AUDIT_FILTER_EXIT &&
> 155 krule->listnr != AUDIT_FILTER_URING_EXIT) ||
156 krule->inode_f || krule->watch || krule->tree ||
157 (f->op != Audit_equal && f->op != Audit_not_equal))
158 return -EINVAL;
159
160 krule->inode_f = f;
161 return 0;
162 }
163
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
Hi Richard,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on pcmoore-audit/next]
[also build test ERROR on v5.13-rc4 next-20210528]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url: https://github.com/0day-ci/linux/commits/Richard-Guy-Briggs/audit-add-filtering-for-io_uring-records-addendum/20210531-214941
base: https://git.kernel.org/pub/scm/linux/kernel/git/pcmoore/audit.git next
config: arm64-randconfig-r021-20210531 (attached as .config)
compiler: clang version 13.0.0 (https://github.com/llvm/llvm-project bc6799f2f79f0ae87e9f1ebf9d25ba799fbd25a9)
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# install arm64 cross compiling tool for clang build
# apt-get install binutils-aarch64-linux-gnu
# https://github.com/0day-ci/linux/commit/07a3e22a2f984838bc98b43b58e8ef08e9353483
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Richard-Guy-Briggs/audit-add-filtering-for-io_uring-records-addendum/20210531-214941
git checkout 07a3e22a2f984838bc98b43b58e8ef08e9353483
# save the attached .config to linux build tree
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=arm64
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
All errors (new ones prefixed by >>):
>> kernel/auditfilter.c:155:24: error: use of undeclared identifier 'AUDIT_FILTER_URING_EXIT'
krule->listnr != AUDIT_FILTER_URING_EXIT) ||
^
kernel/auditfilter.c:252:7: error: use of undeclared identifier 'AUDIT_FILTER_URING_EXIT'
case AUDIT_FILTER_URING_EXIT:
^
kernel/auditfilter.c:986:28: error: use of undeclared identifier 'AUDIT_FILTER_URING_EXIT'
entry->rule.listnr == AUDIT_FILTER_URING_EXIT) {
^
3 errors generated.
--
>> kernel/audit_watch.c:187:24: error: use of undeclared identifier 'AUDIT_FILTER_URING_EXIT'
krule->listnr != AUDIT_FILTER_URING_EXIT) ||
^
1 error generated.
--
>> kernel/audit_tree.c:731:23: error: use of undeclared identifier 'AUDIT_FILTER_URING_EXIT'
rule->listnr != AUDIT_FILTER_URING_EXIT) ||
^
1 error generated.
vim +/AUDIT_FILTER_URING_EXIT +155 kernel/auditfilter.c
149
150 /* Translate an inode field to kernel representation. */
151 static inline int audit_to_inode(struct audit_krule *krule,
152 struct audit_field *f)
153 {
154 if ((krule->listnr != AUDIT_FILTER_EXIT &&
> 155 krule->listnr != AUDIT_FILTER_URING_EXIT) ||
156 krule->inode_f || krule->watch || krule->tree ||
157 (f->op != Audit_equal && f->op != Audit_not_equal))
158 return -EINVAL;
159
160 krule->inode_f = f;
161 return 0;
162 }
163
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
On Mon, May 31, 2021 at 9:45 AM Richard Guy Briggs <rgb@redhat.com> wrote: > > The commit ("audit: add filtering for io_uring records") added support for > filtering io_uring operations. > > Add checks to the audit io_uring filtering code for directory and path watches, > and to keep the list counts consistent. > > Signed-off-by: Richard Guy Briggs <rgb@redhat.com> > --- > kernel/audit_tree.c | 3 ++- > kernel/audit_watch.c | 3 ++- > kernel/auditfilter.c | 7 +++++-- > 3 files changed, 9 insertions(+), 4 deletions(-) Thanks for pointing these omissions out in the original patch. When a patch has obvious problems generally people just provide feedback and the patch author incorporates the fixes; this helps ensure we don't merge known broken patches, helping preserve `git bisect`. Do you mind if I incorporate these suggestions, and the one in patch 2/2, into the filtering patch in the original RFC patchset? I'll add a 'thank you' comment in the commit description as I did to the other patch where you provided feedback. I feel that is the proper way to handle this. > diff --git a/kernel/audit_tree.c b/kernel/audit_tree.c > index 6c91902f4f45..2be285c2f069 100644 > --- a/kernel/audit_tree.c > +++ b/kernel/audit_tree.c > @@ -727,7 +727,8 @@ int audit_make_tree(struct audit_krule *rule, char *pathname, u32 op) > { > > if (pathname[0] != '/' || > - rule->listnr != AUDIT_FILTER_EXIT || > + (rule->listnr != AUDIT_FILTER_EXIT && > + rule->listnr != AUDIT_FILTER_URING_EXIT) || > op != Audit_equal || > rule->inode_f || rule->watch || rule->tree) > return -EINVAL; > diff --git a/kernel/audit_watch.c b/kernel/audit_watch.c > index 2acf7ca49154..698b62b4a2ec 100644 > --- a/kernel/audit_watch.c > +++ b/kernel/audit_watch.c > @@ -183,7 +183,8 @@ int audit_to_watch(struct audit_krule *krule, char *path, int len, u32 op) > return -EOPNOTSUPP; > > if (path[0] != '/' || path[len-1] == '/' || > - krule->listnr != AUDIT_FILTER_EXIT || > + (krule->listnr != AUDIT_FILTER_EXIT && > + krule->listnr != AUDIT_FILTER_URING_EXIT) || > op != Audit_equal || > krule->inode_f || krule->watch || krule->tree) > return -EINVAL; > diff --git a/kernel/auditfilter.c b/kernel/auditfilter.c > index c21119c00504..bcdedfd1088c 100644 > --- a/kernel/auditfilter.c > +++ b/kernel/auditfilter.c > @@ -153,7 +153,8 @@ char *audit_unpack_string(void **bufp, size_t *remain, size_t len) > static inline int audit_to_inode(struct audit_krule *krule, > struct audit_field *f) > { > - if (krule->listnr != AUDIT_FILTER_EXIT || > + if ((krule->listnr != AUDIT_FILTER_EXIT && > + krule->listnr != AUDIT_FILTER_URING_EXIT) || > krule->inode_f || krule->watch || krule->tree || > (f->op != Audit_equal && f->op != Audit_not_equal)) > return -EINVAL; > @@ -250,6 +251,7 @@ static inline struct audit_entry *audit_to_entry_common(struct audit_rule_data * > pr_err("AUDIT_FILTER_ENTRY is deprecated\n"); > goto exit_err; > case AUDIT_FILTER_EXIT: > + case AUDIT_FILTER_URING_EXIT: > case AUDIT_FILTER_TASK: > #endif > case AUDIT_FILTER_USER: > @@ -982,7 +984,8 @@ static inline int audit_add_rule(struct audit_entry *entry) > } > > entry->rule.prio = ~0ULL; > - if (entry->rule.listnr == AUDIT_FILTER_EXIT) { > + if (entry->rule.listnr == AUDIT_FILTER_EXIT || > + entry->rule.listnr == AUDIT_FILTER_URING_EXIT) { > if (entry->rule.flags & AUDIT_FILTER_PREPEND) > entry->rule.prio = ++prio_high; > else > -- > 2.27.0
On 2021-06-07 19:15, Paul Moore wrote: > On Mon, May 31, 2021 at 9:45 AM Richard Guy Briggs <rgb@redhat.com> wrote: > > The commit ("audit: add filtering for io_uring records") added support for > > filtering io_uring operations. > > > > Add checks to the audit io_uring filtering code for directory and path watches, > > and to keep the list counts consistent. > > > > Signed-off-by: Richard Guy Briggs <rgb@redhat.com> > > --- > > kernel/audit_tree.c | 3 ++- > > kernel/audit_watch.c | 3 ++- > > kernel/auditfilter.c | 7 +++++-- > > 3 files changed, 9 insertions(+), 4 deletions(-) > > Thanks for pointing these omissions out in the original patch. When a > patch has obvious problems generally people just provide feedback and > the patch author incorporates the fixes; this helps ensure we don't > merge known broken patches, helping preserve `git bisect`. > > Do you mind if I incorporate these suggestions, and the one in patch > 2/2, into the filtering patch in the original RFC patchset? I'll add > a 'thank you' comment in the commit description as I did to the other > patch where you provided feedback. I feel that is the proper way to > handle this. I should have been more explicit. The intent was to have the fixes incorporated directly into your patches since they aren't committed in any public tree yet, exactly for bisect reasons. I didn't add a "fixes:" tag because it had no public commit hash, but could/should have instead made a note about it or even used the "fixup:" subject tag. Posting using the thread as the "in-reply-to:" for this patchset was the simplest and clearest way to demonstrate the intent and check they were correct and I was too lazy to add a cover letter explaining that. There is also a Co-developed-by: tag that could be used if you feel that is appropriate, now that you mention it, but that appears to imply a much stronger equal contribution. > > diff --git a/kernel/audit_tree.c b/kernel/audit_tree.c > > index 6c91902f4f45..2be285c2f069 100644 > > --- a/kernel/audit_tree.c > > +++ b/kernel/audit_tree.c > > @@ -727,7 +727,8 @@ int audit_make_tree(struct audit_krule *rule, char *pathname, u32 op) > > { > > > > if (pathname[0] != '/' || > > - rule->listnr != AUDIT_FILTER_EXIT || > > + (rule->listnr != AUDIT_FILTER_EXIT && > > + rule->listnr != AUDIT_FILTER_URING_EXIT) || > > op != Audit_equal || > > rule->inode_f || rule->watch || rule->tree) > > return -EINVAL; > > diff --git a/kernel/audit_watch.c b/kernel/audit_watch.c > > index 2acf7ca49154..698b62b4a2ec 100644 > > --- a/kernel/audit_watch.c > > +++ b/kernel/audit_watch.c > > @@ -183,7 +183,8 @@ int audit_to_watch(struct audit_krule *krule, char *path, int len, u32 op) > > return -EOPNOTSUPP; > > > > if (path[0] != '/' || path[len-1] == '/' || > > - krule->listnr != AUDIT_FILTER_EXIT || > > + (krule->listnr != AUDIT_FILTER_EXIT && > > + krule->listnr != AUDIT_FILTER_URING_EXIT) || > > op != Audit_equal || > > krule->inode_f || krule->watch || krule->tree) > > return -EINVAL; > > diff --git a/kernel/auditfilter.c b/kernel/auditfilter.c > > index c21119c00504..bcdedfd1088c 100644 > > --- a/kernel/auditfilter.c > > +++ b/kernel/auditfilter.c > > @@ -153,7 +153,8 @@ char *audit_unpack_string(void **bufp, size_t *remain, size_t len) > > static inline int audit_to_inode(struct audit_krule *krule, > > struct audit_field *f) > > { > > - if (krule->listnr != AUDIT_FILTER_EXIT || > > + if ((krule->listnr != AUDIT_FILTER_EXIT && > > + krule->listnr != AUDIT_FILTER_URING_EXIT) || > > krule->inode_f || krule->watch || krule->tree || > > (f->op != Audit_equal && f->op != Audit_not_equal)) > > return -EINVAL; > > @@ -250,6 +251,7 @@ static inline struct audit_entry *audit_to_entry_common(struct audit_rule_data * > > pr_err("AUDIT_FILTER_ENTRY is deprecated\n"); > > goto exit_err; > > case AUDIT_FILTER_EXIT: > > + case AUDIT_FILTER_URING_EXIT: > > case AUDIT_FILTER_TASK: > > #endif > > case AUDIT_FILTER_USER: > > @@ -982,7 +984,8 @@ static inline int audit_add_rule(struct audit_entry *entry) > > } > > > > entry->rule.prio = ~0ULL; > > - if (entry->rule.listnr == AUDIT_FILTER_EXIT) { > > + if (entry->rule.listnr == AUDIT_FILTER_EXIT || > > + entry->rule.listnr == AUDIT_FILTER_URING_EXIT) { > > if (entry->rule.flags & AUDIT_FILTER_PREPEND) > > entry->rule.prio = ++prio_high; > > else > > paul moore - RGB -- Richard Guy Briggs <rgb@redhat.com> Sr. S/W Engineer, Kernel Security, Base Operating Systems Remote, Ottawa, Red Hat Canada IRC: rgb, SunRaycer Voice: +1.647.777.2635, Internal: (81) 32635
On Tue, Jun 8, 2021 at 8:55 AM Richard Guy Briggs <rgb@redhat.com> wrote: > I should have been more explicit. The intent was to have the fixes > incorporated directly into your patches since they aren't committed in > any public tree yet, exactly for bisect reasons ... No worries, thanks for the clarification Richard. I just wanted to make sure since the contribution format was a bit unusual given the context :) Regardless, thanks again for the feedback, I'll get this incorporated.
diff --git a/kernel/audit_tree.c b/kernel/audit_tree.c index 6c91902f4f45..2be285c2f069 100644 --- a/kernel/audit_tree.c +++ b/kernel/audit_tree.c @@ -727,7 +727,8 @@ int audit_make_tree(struct audit_krule *rule, char *pathname, u32 op) { if (pathname[0] != '/' || - rule->listnr != AUDIT_FILTER_EXIT || + (rule->listnr != AUDIT_FILTER_EXIT && + rule->listnr != AUDIT_FILTER_URING_EXIT) || op != Audit_equal || rule->inode_f || rule->watch || rule->tree) return -EINVAL; diff --git a/kernel/audit_watch.c b/kernel/audit_watch.c index 2acf7ca49154..698b62b4a2ec 100644 --- a/kernel/audit_watch.c +++ b/kernel/audit_watch.c @@ -183,7 +183,8 @@ int audit_to_watch(struct audit_krule *krule, char *path, int len, u32 op) return -EOPNOTSUPP; if (path[0] != '/' || path[len-1] == '/' || - krule->listnr != AUDIT_FILTER_EXIT || + (krule->listnr != AUDIT_FILTER_EXIT && + krule->listnr != AUDIT_FILTER_URING_EXIT) || op != Audit_equal || krule->inode_f || krule->watch || krule->tree) return -EINVAL; diff --git a/kernel/auditfilter.c b/kernel/auditfilter.c index c21119c00504..bcdedfd1088c 100644 --- a/kernel/auditfilter.c +++ b/kernel/auditfilter.c @@ -153,7 +153,8 @@ char *audit_unpack_string(void **bufp, size_t *remain, size_t len) static inline int audit_to_inode(struct audit_krule *krule, struct audit_field *f) { - if (krule->listnr != AUDIT_FILTER_EXIT || + if ((krule->listnr != AUDIT_FILTER_EXIT && + krule->listnr != AUDIT_FILTER_URING_EXIT) || krule->inode_f || krule->watch || krule->tree || (f->op != Audit_equal && f->op != Audit_not_equal)) return -EINVAL; @@ -250,6 +251,7 @@ static inline struct audit_entry *audit_to_entry_common(struct audit_rule_data * pr_err("AUDIT_FILTER_ENTRY is deprecated\n"); goto exit_err; case AUDIT_FILTER_EXIT: + case AUDIT_FILTER_URING_EXIT: case AUDIT_FILTER_TASK: #endif case AUDIT_FILTER_USER: @@ -982,7 +984,8 @@ static inline int audit_add_rule(struct audit_entry *entry) } entry->rule.prio = ~0ULL; - if (entry->rule.listnr == AUDIT_FILTER_EXIT) { + if (entry->rule.listnr == AUDIT_FILTER_EXIT || + entry->rule.listnr == AUDIT_FILTER_URING_EXIT) { if (entry->rule.flags & AUDIT_FILTER_PREPEND) entry->rule.prio = ++prio_high; else
The commit ("audit: add filtering for io_uring records") added support for filtering io_uring operations. Add checks to the audit io_uring filtering code for directory and path watches, and to keep the list counts consistent. Signed-off-by: Richard Guy Briggs <rgb@redhat.com> --- kernel/audit_tree.c | 3 ++- kernel/audit_watch.c | 3 ++- kernel/auditfilter.c | 7 +++++-- 3 files changed, 9 insertions(+), 4 deletions(-)