On Mon, Oct 2, 2023 at 10:58 AM Stephen Smalley
<stephen.smalley.work@xxxxxxxxx> wrote:
>
> On Fri, Sep 29, 2023 at 3:56 PM Jacob Satterfield
> <jsatterfield.linux@xxxxxxxxx> wrote:
> >
> > Similar to the list_for_each macros in list.h, this patch adds two
> > macros that iterates an avtab_node linked list (avtab_chain_for_each and
> > avtab_chain_for_each_prev). This has two benefits: it reduces the amount
> > of duplicative code for iteration and it makes changes to the underlying
> > hashtable data structure easier as there are fewer places to update.
>
> You will need/want an equivalent to list_for_each_safe() or open-code
> it to handle avtab_destroy() below.
>
> >
> > Signed-off-by: Jacob Satterfield <jsatterfield.linux@xxxxxxxxx>
> > ---
> > security/selinux/ss/avtab.c | 40 ++++++++++++++++---------------------
> > 1 file changed, 17 insertions(+), 23 deletions(-)
> >
> > diff --git a/security/selinux/ss/avtab.c b/security/selinux/ss/avtab.c
> > index 1cd4fed30bf7..e8046eda7140 100644
> > --- a/security/selinux/ss/avtab.c
> > +++ b/security/selinux/ss/avtab.c
> > @@ -223,20 +223,17 @@ avtab_search_node_next(struct avtab_node *node, u16 specified)
> > void avtab_destroy(struct avtab *h)
> > {
> > u32 i;
> > - struct avtab_node *cur, *temp;
> > + struct avtab_node *cur;
> >
> > if (!h)
> > return;
> >
> > for (i = 0; i < h->nslot; i++) {
> > - cur = h->htable[i];
> > - while (cur) {
> > - temp = cur;
> > - cur = cur->next;
> > - if (temp->key.specified & AVTAB_XPERMS)
> > + avtab_chain_for_each(cur, h, i) {
> > + if (cur->key.specified & AVTAB_XPERMS)
> > kmem_cache_free(avtab_xperms_cachep,
> > - temp->datum.u.xperms);
> > - kmem_cache_free(avtab_node_cachep, temp);
> > + cur->datum.u.xperms);
> > + kmem_cache_free(avtab_node_cachep, cur);
> > }
> > }
> > kvfree(h->htable);
>
> This requires an avtab_chain_for_each_safe() or similar since it frees the node.
Great catch! Since this separate macro would only have one invocation,
it would make the code less readable without any benefit. I have
reverted the changes to avtab_destroy() for the next spin.
Thanks!
-Jacob
