[apparmor] [PATCH v2] APPARMOR: add sid to profile mapping and sid recycling

wzt wzt wzt.wzt at gmail.com
Wed Dec 1 03:16:40 GMT 2010


>> +u32 sid_bitmap[AA_SID_BITMAP_SIZE] = {0};
>
> No need to initialize static variables with 0 or NULL.
>
> We have BITS_PER_LONG.
> Why not to use "unsigned long" instead of "u32" so that we can use ffz()?
>

yes, will use  BITS_PER_LONG in the next version.

>> +     /* find the first zero bit in the sid_bitmap array */
>> +     spin_lock(&aa_sid_hash_table->lock);
>> +     for (i = 0; i < AA_SID_BITMAP_SIZE; i++) {
>> +             for (j = 0; j < 32; j++) {
>> +                     if (!(sid_bitmap[i] & (1 << j))) {
>> +                             /* convert offset to sid */
>> +                             sid = i * 32 + j;
>> +                             goto alloc_ok;
>> +                     }
>> +             }
>> +     }
>> +     spin_unlock(&aa_sid_hash_table->lock);
>
> If you use sid values for only 0 - 32767 range (rather than full u32 range),
> you can allocate
>
>        char sid_map[32768 + 1];
>
> and find an available sid by
>
>        spin_lock(&aa_sid_hash_table->lock);
>        sid = strlen(sid_map);
>        spin_unlock(&aa_sid_hash_table->lock);
>
> .
>

May be we can use a global variable like aa_last_sid, aa_alloc_sid()
will get the newest sid in the sid_bitmap arrary first, if aa_last_sid
if bigger than AA_SID_BITMAP_SIZE*BITS_PER_LONG, than start a loop
looks up for a freed sid, so the logic can change like bellow:

spin_lock(&aa_sid_hash_table->lock);
if (aa_last_sid < AA_SID_BITMAP_SIZE*BITS_PER_LONG) {
    sid = aa_last_sid ++;
    goto alloc_ok;
}

 for (i = 0; i < AA_SID_BITMAP_SIZE; i++) {
     for (j = 0; j < 32; j++) {
         if (!(sid_bitmap[i] & (1 << j))) {
             /* convert offset to sid */
             sid = i * 32 + j;
            goto alloc_ok;
        }
   }
}
spin_unlock(&aa_sid_hash_table->lock);
kfree(node);
return -1;

alloc_ok:
    node->sid = sid;
    hash_value = AA_SIDTAB_HASH(sid);
    list_add_tail(&(node->list), &aa_sid_hash_table->sid_list[hash_value]);
    set_sid_bitmap(sid);
     spin_unlock(&aa_sid_hash_table->lock);
     return sid;



More information about the AppArmor mailing list