Skip to content

Commit a9eb185

Browse files
committed
apparmor: fix x_table_lookup when stacking is not the first entry
x_table_lookup currently does stacking during label_parse() if the target specifies a stack but its only caller ensures that it will never be used with stacking. Refactor to slightly simplify the code in x_to_label(), this also fixes a long standing problem where x_to_labels check on stacking is only on the first element to the table option list, instead of the element that is found and used. Signed-off-by: John Johansen <john.johansen@canonical.com>
1 parent 84c455d commit a9eb185

File tree

1 file changed

+29
-23
lines changed

1 file changed

+29
-23
lines changed

security/apparmor/domain.c

Lines changed: 29 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -509,6 +509,7 @@ static const char *next_name(int xtype, const char *name)
509509
* @name: returns: name tested to find label (NOT NULL)
510510
*
511511
* Returns: refcounted label, or NULL on failure (MAYBE NULL)
512+
* @name will always be set with the last name tried
512513
*/
513514
struct aa_label *x_table_lookup(struct aa_profile *profile, u32 xindex,
514515
const char **name)
@@ -518,32 +519,35 @@ struct aa_label *x_table_lookup(struct aa_profile *profile, u32 xindex,
518519
struct aa_label *label = NULL;
519520
u32 xtype = xindex & AA_X_TYPE_MASK;
520521
int index = xindex & AA_X_INDEX_MASK;
522+
const char *next;
521523

522524
AA_BUG(!name);
523525

524526
/* index is guaranteed to be in range, validated at load time */
525527
/* TODO: move lookup parsing to unpack time so this is a straight
526528
* index into the resultant label
527529
*/
528-
for (*name = rules->file->trans.table[index]; !label && *name;
529-
*name = next_name(xtype, *name)) {
530+
for (next = rules->file->trans.table[index]; next;
531+
next = next_name(xtype, next)) {
532+
const char *lookup = (*next == '&') ? next + 1 : next;
533+
*name = next;
530534
if (xindex & AA_X_CHILD) {
531-
struct aa_profile *new_profile;
532-
/* release by caller */
533-
new_profile = aa_find_child(profile, *name);
534-
if (new_profile)
535-
label = &new_profile->label;
535+
/* TODO: switich to parse to get stack of child */
536+
struct aa_profile *new = aa_find_child(profile, lookup);
537+
538+
if (new)
539+
/* release by caller */
540+
return &new->label;
536541
continue;
537542
}
538-
label = aa_label_parse(&profile->label, *name, GFP_KERNEL,
543+
label = aa_label_parse(&profile->label, lookup, GFP_KERNEL,
539544
true, false);
540-
if (IS_ERR(label))
541-
label = NULL;
545+
if (!IS_ERR_OR_NULL(label))
546+
/* release by caller */
547+
return label;
542548
}
543549

544-
/* released by caller */
545-
546-
return label;
550+
return NULL;
547551
}
548552

549553
/**
@@ -568,9 +572,9 @@ static struct aa_label *x_to_label(struct aa_profile *profile,
568572
struct aa_ruleset *rules = list_first_entry(&profile->rules,
569573
typeof(*rules), list);
570574
struct aa_label *new = NULL;
575+
struct aa_label *stack = NULL;
571576
struct aa_ns *ns = profile->ns;
572577
u32 xtype = xindex & AA_X_TYPE_MASK;
573-
const char *stack = NULL;
574578

575579
switch (xtype) {
576580
case AA_X_NONE:
@@ -579,13 +583,14 @@ static struct aa_label *x_to_label(struct aa_profile *profile,
579583
break;
580584
case AA_X_TABLE:
581585
/* TODO: fix when perm mapping done at unload */
582-
stack = rules->file->trans.table[xindex & AA_X_INDEX_MASK];
583-
if (*stack != '&') {
584-
/* released by caller */
585-
new = x_table_lookup(profile, xindex, lookupname);
586-
stack = NULL;
586+
/* released by caller
587+
* if null for both stack and direct want to try fallback
588+
*/
589+
new = x_table_lookup(profile, xindex, lookupname);
590+
if (!new || **lookupname != '&')
587591
break;
588-
}
592+
stack = new;
593+
new = NULL;
589594
fallthrough; /* to X_NAME */
590595
case AA_X_NAME:
591596
if (xindex & AA_X_CHILD)
@@ -600,6 +605,7 @@ static struct aa_label *x_to_label(struct aa_profile *profile,
600605
break;
601606
}
602607

608+
/* fallback transition check */
603609
if (!new) {
604610
if (xindex & AA_X_INHERIT) {
605611
/* (p|c|n)ix - don't change profile but do
@@ -618,12 +624,12 @@ static struct aa_label *x_to_label(struct aa_profile *profile,
618624
/* base the stack on post domain transition */
619625
struct aa_label *base = new;
620626

621-
new = aa_label_parse(base, stack, GFP_KERNEL, true, false);
622-
if (IS_ERR(new))
623-
new = NULL;
627+
new = aa_label_merge(base, stack, GFP_KERNEL);
628+
/* null on error */
624629
aa_put_label(base);
625630
}
626631

632+
aa_put_label(stack);
627633
/* released by caller */
628634
return new;
629635
}

0 commit comments

Comments
 (0)