* [PATCH V4 (was V6) 0/2] audit: rebalance and remove extra layers of watch references
@ 2015-08-01 19:41 ` Richard Guy Briggs
0 siblings, 0 replies; 7+ messages in thread
From: Richard Guy Briggs @ 2015-08-01 19:41 UTC (permalink / raw)
To: linux-audit, linux-kernel; +Cc: Richard Guy Briggs
While working on the audit by executable path feature, it was discovered that
watches and parent references were slightly imbalanced and deeper than
necessary.
Only bump up references when they are actually used and decrease when removed.
v4: Eliminate unnecessary gotos and call return directly.
Expand patch description with answer to mailing list query.
Richard Guy Briggs (2):
audit: eliminate unnecessary extra layer of watch references
audit: eliminate unnecessary extra layer of watch parent references
kernel/audit_watch.c | 11 ++++-------
kernel/auditfilter.c | 16 +++-------------
2 files changed, 7 insertions(+), 20 deletions(-)
^ permalink raw reply [flat|nested] 7+ messages in thread* [PATCH V4 (was V6) 0/2] audit: rebalance and remove extra layers of watch references @ 2015-08-01 19:41 ` Richard Guy Briggs 0 siblings, 0 replies; 7+ messages in thread From: Richard Guy Briggs @ 2015-08-01 19:41 UTC (permalink / raw) To: linux-audit, linux-kernel; +Cc: Richard Guy Briggs, sgrubb, pmoore, eparis While working on the audit by executable path feature, it was discovered that watches and parent references were slightly imbalanced and deeper than necessary. Only bump up references when they are actually used and decrease when removed. v4: Eliminate unnecessary gotos and call return directly. Expand patch description with answer to mailing list query. Richard Guy Briggs (2): audit: eliminate unnecessary extra layer of watch references audit: eliminate unnecessary extra layer of watch parent references kernel/audit_watch.c | 11 ++++------- kernel/auditfilter.c | 16 +++------------- 2 files changed, 7 insertions(+), 20 deletions(-) ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH V4 (was V6) 1/2] audit: eliminate unnecessary extra layer of watch references 2015-08-01 19:41 ` Richard Guy Briggs @ 2015-08-01 19:41 ` Richard Guy Briggs -1 siblings, 0 replies; 7+ messages in thread From: Richard Guy Briggs @ 2015-08-01 19:41 UTC (permalink / raw) To: linux-audit, linux-kernel; +Cc: Richard Guy Briggs The audit watch count was imbalanced, adding an unnecessary layer of watch references. Only add the second reference when it is added to a parent. Signed-off-by: Richard Guy Briggs <rgb@redhat.com> --- kernel/audit_watch.c | 5 ++--- kernel/auditfilter.c | 16 +++------------- 2 files changed, 5 insertions(+), 16 deletions(-) diff --git a/kernel/audit_watch.c b/kernel/audit_watch.c index 6e30024..f33f54c 100644 --- a/kernel/audit_watch.c +++ b/kernel/audit_watch.c @@ -203,7 +203,6 @@ int audit_to_watch(struct audit_krule *krule, char *path, int len, u32 op) if (IS_ERR(watch)) return PTR_ERR(watch); - audit_get_watch(watch); krule->watch = watch; return 0; @@ -387,8 +386,7 @@ static void audit_add_to_parent(struct audit_krule *krule, watch_found = 1; - /* put krule's and initial refs to temporary watch */ - audit_put_watch(watch); + /* put krule's ref to temporary watch */ audit_put_watch(watch); audit_get_watch(w); @@ -400,6 +398,7 @@ static void audit_add_to_parent(struct audit_krule *krule, audit_get_parent(parent); watch->parent = parent; + audit_get_watch(watch); list_add(&watch->wlist, &parent->watches); } list_add(&krule->rlist, &watch->rules); diff --git a/kernel/auditfilter.c b/kernel/auditfilter.c index 72e1660..4cb9b44 100644 --- a/kernel/auditfilter.c +++ b/kernel/auditfilter.c @@ -549,8 +549,6 @@ exit_nofree: return entry; exit_free: - if (entry->rule.watch) - audit_put_watch(entry->rule.watch); /* matches initial get */ if (entry->rule.tree) audit_put_tree(entry->rule.tree); /* that's the temporary one */ audit_free_rule(entry); @@ -881,7 +879,7 @@ static inline int audit_add_rule(struct audit_entry *entry) /* normally audit_add_tree_rule() will free it on failure */ if (tree) audit_put_tree(tree); - goto error; + return err; } if (watch) { @@ -895,14 +893,14 @@ static inline int audit_add_rule(struct audit_entry *entry) */ if (tree) audit_put_tree(tree); - goto error; + return err; } } if (tree) { err = audit_add_tree_rule(&entry->rule); if (err) { mutex_unlock(&audit_filter_mutex); - goto error; + return err; } } @@ -933,11 +931,6 @@ static inline int audit_add_rule(struct audit_entry *entry) #endif mutex_unlock(&audit_filter_mutex); - return 0; - -error: - if (watch) - audit_put_watch(watch); /* tmp watch, matches initial get */ return err; } @@ -945,7 +938,6 @@ error: static inline int audit_del_rule(struct audit_entry *entry) { struct audit_entry *e; - struct audit_watch *watch = entry->rule.watch; struct audit_tree *tree = entry->rule.tree; struct list_head *list; int ret = 0; @@ -986,8 +978,6 @@ static inline int audit_del_rule(struct audit_entry *entry) mutex_unlock(&audit_filter_mutex); out: - if (watch) - audit_put_watch(watch); /* match initial get */ if (tree) audit_put_tree(tree); /* that's the temporary one */ -- 1.7.1 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH V4 (was V6) 1/2] audit: eliminate unnecessary extra layer of watch references @ 2015-08-01 19:41 ` Richard Guy Briggs 0 siblings, 0 replies; 7+ messages in thread From: Richard Guy Briggs @ 2015-08-01 19:41 UTC (permalink / raw) To: linux-audit, linux-kernel; +Cc: Richard Guy Briggs, sgrubb, pmoore, eparis The audit watch count was imbalanced, adding an unnecessary layer of watch references. Only add the second reference when it is added to a parent. Signed-off-by: Richard Guy Briggs <rgb@redhat.com> --- kernel/audit_watch.c | 5 ++--- kernel/auditfilter.c | 16 +++------------- 2 files changed, 5 insertions(+), 16 deletions(-) diff --git a/kernel/audit_watch.c b/kernel/audit_watch.c index 6e30024..f33f54c 100644 --- a/kernel/audit_watch.c +++ b/kernel/audit_watch.c @@ -203,7 +203,6 @@ int audit_to_watch(struct audit_krule *krule, char *path, int len, u32 op) if (IS_ERR(watch)) return PTR_ERR(watch); - audit_get_watch(watch); krule->watch = watch; return 0; @@ -387,8 +386,7 @@ static void audit_add_to_parent(struct audit_krule *krule, watch_found = 1; - /* put krule's and initial refs to temporary watch */ - audit_put_watch(watch); + /* put krule's ref to temporary watch */ audit_put_watch(watch); audit_get_watch(w); @@ -400,6 +398,7 @@ static void audit_add_to_parent(struct audit_krule *krule, audit_get_parent(parent); watch->parent = parent; + audit_get_watch(watch); list_add(&watch->wlist, &parent->watches); } list_add(&krule->rlist, &watch->rules); diff --git a/kernel/auditfilter.c b/kernel/auditfilter.c index 72e1660..4cb9b44 100644 --- a/kernel/auditfilter.c +++ b/kernel/auditfilter.c @@ -549,8 +549,6 @@ exit_nofree: return entry; exit_free: - if (entry->rule.watch) - audit_put_watch(entry->rule.watch); /* matches initial get */ if (entry->rule.tree) audit_put_tree(entry->rule.tree); /* that's the temporary one */ audit_free_rule(entry); @@ -881,7 +879,7 @@ static inline int audit_add_rule(struct audit_entry *entry) /* normally audit_add_tree_rule() will free it on failure */ if (tree) audit_put_tree(tree); - goto error; + return err; } if (watch) { @@ -895,14 +893,14 @@ static inline int audit_add_rule(struct audit_entry *entry) */ if (tree) audit_put_tree(tree); - goto error; + return err; } } if (tree) { err = audit_add_tree_rule(&entry->rule); if (err) { mutex_unlock(&audit_filter_mutex); - goto error; + return err; } } @@ -933,11 +931,6 @@ static inline int audit_add_rule(struct audit_entry *entry) #endif mutex_unlock(&audit_filter_mutex); - return 0; - -error: - if (watch) - audit_put_watch(watch); /* tmp watch, matches initial get */ return err; } @@ -945,7 +938,6 @@ error: static inline int audit_del_rule(struct audit_entry *entry) { struct audit_entry *e; - struct audit_watch *watch = entry->rule.watch; struct audit_tree *tree = entry->rule.tree; struct list_head *list; int ret = 0; @@ -986,8 +978,6 @@ static inline int audit_del_rule(struct audit_entry *entry) mutex_unlock(&audit_filter_mutex); out: - if (watch) - audit_put_watch(watch); /* match initial get */ if (tree) audit_put_tree(tree); /* that's the temporary one */ -- 1.7.1 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH V4 (was V6) 1/2] audit: eliminate unnecessary extra layer of watch references 2015-08-01 19:41 ` Richard Guy Briggs (?) @ 2015-08-04 22:27 ` Paul Moore -1 siblings, 0 replies; 7+ messages in thread From: Paul Moore @ 2015-08-04 22:27 UTC (permalink / raw) To: Richard Guy Briggs; +Cc: linux-audit, linux-kernel, sgrubb, eparis On Saturday, August 01, 2015 03:41:12 PM Richard Guy Briggs wrote: > The audit watch count was imbalanced, adding an unnecessary layer of watch > references. Only add the second reference when it is added to a parent. > > Signed-off-by: Richard Guy Briggs <rgb@redhat.com> > --- > kernel/audit_watch.c | 5 ++--- > kernel/auditfilter.c | 16 +++------------- > 2 files changed, 5 insertions(+), 16 deletions(-) Merged. I'll push it out as soon as I finish up the reviews tonight. > diff --git a/kernel/audit_watch.c b/kernel/audit_watch.c > index 6e30024..f33f54c 100644 > --- a/kernel/audit_watch.c > +++ b/kernel/audit_watch.c > @@ -203,7 +203,6 @@ int audit_to_watch(struct audit_krule *krule, char > *path, int len, u32 op) if (IS_ERR(watch)) > return PTR_ERR(watch); > > - audit_get_watch(watch); > krule->watch = watch; > > return 0; > @@ -387,8 +386,7 @@ static void audit_add_to_parent(struct audit_krule > *krule, > > watch_found = 1; > > - /* put krule's and initial refs to temporary watch */ > - audit_put_watch(watch); > + /* put krule's ref to temporary watch */ > audit_put_watch(watch); > > audit_get_watch(w); > @@ -400,6 +398,7 @@ static void audit_add_to_parent(struct audit_krule > *krule, audit_get_parent(parent); > watch->parent = parent; > > + audit_get_watch(watch); > list_add(&watch->wlist, &parent->watches); > } > list_add(&krule->rlist, &watch->rules); > diff --git a/kernel/auditfilter.c b/kernel/auditfilter.c > index 72e1660..4cb9b44 100644 > --- a/kernel/auditfilter.c > +++ b/kernel/auditfilter.c > @@ -549,8 +549,6 @@ exit_nofree: > return entry; > > exit_free: > - if (entry->rule.watch) > - audit_put_watch(entry->rule.watch); /* matches initial get */ > if (entry->rule.tree) > audit_put_tree(entry->rule.tree); /* that's the temporary one */ > audit_free_rule(entry); > @@ -881,7 +879,7 @@ static inline int audit_add_rule(struct audit_entry > *entry) /* normally audit_add_tree_rule() will free it on failure */ > if (tree) > audit_put_tree(tree); > - goto error; > + return err; > } > > if (watch) { > @@ -895,14 +893,14 @@ static inline int audit_add_rule(struct audit_entry > *entry) */ > if (tree) > audit_put_tree(tree); > - goto error; > + return err; > } > } > if (tree) { > err = audit_add_tree_rule(&entry->rule); > if (err) { > mutex_unlock(&audit_filter_mutex); > - goto error; > + return err; > } > } > > @@ -933,11 +931,6 @@ static inline int audit_add_rule(struct audit_entry > *entry) #endif > mutex_unlock(&audit_filter_mutex); > > - return 0; > - > -error: > - if (watch) > - audit_put_watch(watch); /* tmp watch, matches initial get */ > return err; > } > > @@ -945,7 +938,6 @@ error: > static inline int audit_del_rule(struct audit_entry *entry) > { > struct audit_entry *e; > - struct audit_watch *watch = entry->rule.watch; > struct audit_tree *tree = entry->rule.tree; > struct list_head *list; > int ret = 0; > @@ -986,8 +978,6 @@ static inline int audit_del_rule(struct audit_entry > *entry) mutex_unlock(&audit_filter_mutex); > > out: > - if (watch) > - audit_put_watch(watch); /* match initial get */ > if (tree) > audit_put_tree(tree); /* that's the temporary one */ -- paul moore security @ redhat ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH V4 (was V6) 2/2] audit: eliminate unnecessary extra layer of watch parent references 2015-08-01 19:41 ` Richard Guy Briggs (?) (?) @ 2015-08-01 19:41 ` Richard Guy Briggs 2015-08-04 22:28 ` Paul Moore -1 siblings, 1 reply; 7+ messages in thread From: Richard Guy Briggs @ 2015-08-01 19:41 UTC (permalink / raw) To: linux-audit, linux-kernel; +Cc: Richard Guy Briggs, sgrubb, pmoore, eparis The audit watch parent count was imbalanced, adding an unnecessary layer of watch parent references. Decrement the additional parent reference when a watch is reused, already having a reference to the parent. audit_find_parent() gets a reference to the parent, if the parent is already known. This additional parental reference is not needed if the watch is subsequently found by audit_add_to_parent(), and consumed if the watch does not already exist, so we need to put the parent if the watch is found, and do nothing if this new watch is added to the parent. If the parent wasn't already known, it is created with a refcount of 1 and added to the audit_watch_group, then incremented by one to be subsequently consumed by the newly created watch in audit_add_to_parent(). The rule points to the watch, not to the parent, so the rule's refcount gets bumped, not the parent's. See LKML, 2015-07-16 Signed-off-by: Richard Guy Briggs <rgb@redhat.com> --- kernel/audit_watch.c | 6 ++---- 1 files changed, 2 insertions(+), 4 deletions(-) diff --git a/kernel/audit_watch.c b/kernel/audit_watch.c index f33f54c..8f123d7 100644 --- a/kernel/audit_watch.c +++ b/kernel/audit_watch.c @@ -391,11 +391,12 @@ static void audit_add_to_parent(struct audit_krule *krule, audit_get_watch(w); krule->watch = watch = w; + + audit_put_parent(parent); break; } if (!watch_found) { - audit_get_parent(parent); watch->parent = parent; audit_get_watch(watch); @@ -436,9 +437,6 @@ int audit_add_watch(struct audit_krule *krule, struct list_head **list) audit_add_to_parent(krule, parent); - /* match get in audit_find_parent or audit_init_parent */ - audit_put_parent(parent); - h = audit_hash_ino((u32)watch->ino); *list = &audit_inode_hash[h]; error: -- 1.7.1 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH V4 (was V6) 2/2] audit: eliminate unnecessary extra layer of watch parent references 2015-08-01 19:41 ` [PATCH V4 (was V6) 2/2] audit: eliminate unnecessary extra layer of watch parent references Richard Guy Briggs @ 2015-08-04 22:28 ` Paul Moore 0 siblings, 0 replies; 7+ messages in thread From: Paul Moore @ 2015-08-04 22:28 UTC (permalink / raw) To: Richard Guy Briggs; +Cc: linux-audit, linux-kernel, sgrubb, eparis On Saturday, August 01, 2015 03:41:13 PM Richard Guy Briggs wrote: > The audit watch parent count was imbalanced, adding an unnecessary layer of > watch parent references. Decrement the additional parent reference when a > watch is reused, already having a reference to the parent. > > audit_find_parent() gets a reference to the parent, if the parent is > already known. This additional parental reference is not needed if the > watch is subsequently found by audit_add_to_parent(), and consumed if > the watch does not already exist, so we need to put the parent if the > watch is found, and do nothing if this new watch is added to the parent. > > If the parent wasn't already known, it is created with a refcount of 1 > and added to the audit_watch_group, then incremented by one to be > subsequently consumed by the newly created watch in > audit_add_to_parent(). > > The rule points to the watch, not to the parent, so the rule's refcount > gets bumped, not the parent's. > > See LKML, 2015-07-16 > > Signed-off-by: Richard Guy Briggs <rgb@redhat.com> > --- > kernel/audit_watch.c | 6 ++---- > 1 files changed, 2 insertions(+), 4 deletions(-) Merged. > diff --git a/kernel/audit_watch.c b/kernel/audit_watch.c > index f33f54c..8f123d7 100644 > --- a/kernel/audit_watch.c > +++ b/kernel/audit_watch.c > @@ -391,11 +391,12 @@ static void audit_add_to_parent(struct audit_krule > *krule, > > audit_get_watch(w); > krule->watch = watch = w; > + > + audit_put_parent(parent); > break; > } > > if (!watch_found) { > - audit_get_parent(parent); > watch->parent = parent; > > audit_get_watch(watch); > @@ -436,9 +437,6 @@ int audit_add_watch(struct audit_krule *krule, struct > list_head **list) > > audit_add_to_parent(krule, parent); > > - /* match get in audit_find_parent or audit_init_parent */ > - audit_put_parent(parent); > - > h = audit_hash_ino((u32)watch->ino); > *list = &audit_inode_hash[h]; > error: -- paul moore security @ redhat ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2015-08-04 22:28 UTC | newest] Thread overview: 7+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2015-08-01 19:41 [PATCH V4 (was V6) 0/2] audit: rebalance and remove extra layers of watch references Richard Guy Briggs 2015-08-01 19:41 ` Richard Guy Briggs 2015-08-01 19:41 ` [PATCH V4 (was V6) 1/2] audit: eliminate unnecessary extra layer " Richard Guy Briggs 2015-08-01 19:41 ` Richard Guy Briggs 2015-08-04 22:27 ` Paul Moore 2015-08-01 19:41 ` [PATCH V4 (was V6) 2/2] audit: eliminate unnecessary extra layer of watch parent references Richard Guy Briggs 2015-08-04 22:28 ` Paul Moore
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.