Linux kernel mirror (for testing) git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
kernel os linux
1
fork

Configure Feed

Select the types of activity you want to include in your feed.

apparmor: fix loop detection used in conflicting attachment resolution

Conflicting attachment resolution is based on the number of states
traversed to reach an accepting state in the attachment DFA, accounting
for DFA loops traversed during the matching process. However, the loop
counting logic had multiple bugs:

- The inc_wb_pos macro increments both position and length, but length
is supposed to saturate upon hitting buffer capacity, instead of
wrapping around.
- If no revisited state is found when traversing the history, is_loop
would still return true, as if there was a loop found the length of
the history buffer, instead of returning false and signalling that
no loop was found. As a result, the adjustment step of
aa_dfa_leftmatch would sometimes produce negative counts with loop-
free DFAs that traversed enough states.
- The iteration in the is_loop for loop is supposed to stop before
i = wb->len, so the conditional should be < instead of <=.

This patch fixes the above bugs as well as the following nits:
- The count and size fields in struct match_workbuf were not used,
so they can be removed.
- The history buffer in match_workbuf semantically stores aa_state_t
and not unsigned ints, even if aa_state_t is currently unsigned int.
- The local variables in is_loop are counters, and thus should be
unsigned ints instead of aa_state_t's.

Fixes: 21f606610502 ("apparmor: improve overlapping domain attachment resolution")

Signed-off-by: Ryan Lee <ryan.lee@canonical.com>
Co-developed-by: John Johansen <john.johansen@canonical.com>
Signed-off-by: John Johansen <john.johansen@canonical.com>

authored by

Ryan Lee and committed by
John Johansen
a88db916 6c055e62

+12 -15
+1 -4
security/apparmor/include/match.h
··· 140 140 /* This needs to be a power of 2 */ 141 141 #define WB_HISTORY_SIZE 32 142 142 struct match_workbuf { 143 - unsigned int count; 144 143 unsigned int pos; 145 144 unsigned int len; 146 - unsigned int size; /* power of 2, same as history size */ 147 - unsigned int history[WB_HISTORY_SIZE]; 145 + aa_state_t history[WB_HISTORY_SIZE]; 148 146 }; 149 147 #define DEFINE_MATCH_WB(N) \ 150 148 struct match_workbuf N = { \ 151 - .count = 0, \ 152 149 .pos = 0, \ 153 150 .len = 0, \ 154 151 }
+11 -11
security/apparmor/match.c
··· 679 679 return state; 680 680 } 681 681 682 - #define inc_wb_pos(wb) \ 683 - do { \ 682 + #define inc_wb_pos(wb) \ 683 + do { \ 684 684 BUILD_BUG_ON_NOT_POWER_OF_2(WB_HISTORY_SIZE); \ 685 685 wb->pos = (wb->pos + 1) & (WB_HISTORY_SIZE - 1); \ 686 - wb->len = (wb->len + 1) & (WB_HISTORY_SIZE - 1); \ 686 + wb->len = (wb->len + 1) > WB_HISTORY_SIZE ? WB_HISTORY_SIZE : \ 687 + wb->len + 1; \ 687 688 } while (0) 688 689 689 690 /* For DFAs that don't support extended tagging of states */ 691 + /* adjust is only set if is_loop returns true */ 690 692 static bool is_loop(struct match_workbuf *wb, aa_state_t state, 691 693 unsigned int *adjust) 692 694 { 693 - aa_state_t pos = wb->pos; 694 - aa_state_t i; 695 + int pos = wb->pos; 696 + int i; 695 697 696 698 if (wb->history[pos] < state) 697 699 return false; 698 700 699 - for (i = 0; i <= wb->len; i++) { 701 + for (i = 0; i < wb->len; i++) { 700 702 if (wb->history[pos] == state) { 701 703 *adjust = i; 702 704 return true; 703 705 } 704 - if (pos == 0) 705 - pos = WB_HISTORY_SIZE; 706 - pos--; 706 + /* -1 wraps to WB_HISTORY_SIZE - 1 */ 707 + pos = (pos - 1) & (WB_HISTORY_SIZE - 1); 707 708 } 708 709 709 - *adjust = i; 710 - return true; 710 + return false; 711 711 } 712 712 713 713 static aa_state_t leftmatch_fb(struct aa_dfa *dfa, aa_state_t start,