[ I don't know why this static checker warning is showing up only now
two years later... -dan ]
Hello John Johansen,
Commit 2e12c5f06017 ("apparmor: add additional flags to extended
permission.") from Jul 23, 2023 (linux-next), leads to the following
Smatch static checker warning:
security/apparmor/policy_unpack.c:780 unpack_pdb()
error: we previously assumed 'pdb->dfa' could be null (see line 753)
security/apparmor/policy_unpack.c
712 static int unpack_pdb(struct aa_ext *e, struct aa_policydb **policy,
713 bool required_dfa, bool required_trans,
714 const char **info)
715 {
716 struct aa_policydb *pdb;
717 void *pos = e->pos;
718 int i, flags, error = -EPROTO;
719 ssize_t size;
720 u32 version = 0;
721
722 pdb = aa_alloc_pdb(GFP_KERNEL);
723 if (!pdb)
724 return -ENOMEM;
725
726 size = unpack_perms_table(e, &pdb->perms);
727 if (size < 0) {
728 error = size;
729 pdb->perms = NULL;
730 *info = "failed to unpack - perms";
731 goto fail;
732 }
733 pdb->size = size;
734
735 if (pdb->perms) {
736 /* perms table present accept is index */
737 flags = TO_ACCEPT1_FLAG(YYTD_DATA32);
738 if (aa_unpack_u32(e, &version, "permsv") && version > 2)
739 /* accept2 used for dfa flags */
740 flags |= TO_ACCEPT2_FLAG(YYTD_DATA32);
741 } else {
742 /* packed perms in accept1 and accept2 */
743 flags = TO_ACCEPT1_FLAG(YYTD_DATA32) |
744 TO_ACCEPT2_FLAG(YYTD_DATA32);
745 }
746
747 pdb->dfa = unpack_dfa(e, flags);
748 if (IS_ERR(pdb->dfa)) {
749 error = PTR_ERR(pdb->dfa);
750 pdb->dfa = NULL;
751 *info = "failed to unpack - dfa";
752 goto fail;
753 } else if (!pdb->dfa) {
754 if (required_dfa) {
755 *info = "missing required dfa";
756 goto fail;
757 }
Assume required_dfa is false.
758 } else {
759 /*
760 * only unpack the following if a dfa is present
761 *
762 * sadly start was given different names for file and
policydb
763 * but since it is optional we can try both
764 */
765 if (!aa_unpack_u32(e, &pdb->start[0], "start"))
766 /* default start state */
767 pdb->start[0] = DFA_START;
768 if (!aa_unpack_u32(e, &pdb->start[AA_CLASS_FILE],
"dfa_start")) {
769 /* default start state for xmatch and file dfa
*/
770 pdb->start[AA_CLASS_FILE] = DFA_START;
771 } /* setup class index */
772 for (i = AA_CLASS_FILE + 1; i <= AA_CLASS_LAST; i++) {
773 pdb->start[i] = aa_dfa_next(pdb->dfa,
pdb->start[0],
774 i);
775 }
776 }
777
778 if (pdb->perms && version <= 2) {
779 /* add dfa flags table missing in v2 */
--> 780 u32 noents = pdb->dfa->tables[YYTD_ID_ACCEPT]->td_lolen;
^^^^^^^^^^
Potential NULL pointer dereference
781 u16 tdflags =
pdb->dfa->tables[YYTD_ID_ACCEPT]->td_flags;
782 size_t tsize = table_size(noents, tdflags);
783
784 pdb->dfa->tables[YYTD_ID_ACCEPT2] = kvzalloc(tsize,
GFP_KERNEL);
785 if (!pdb->dfa->tables[YYTD_ID_ACCEPT2]) {
786 *info = "failed to alloc dfa flags table";
787 goto out;
788 }
789 pdb->dfa->tables[YYTD_ID_ACCEPT2]->td_lolen = noents;
790 pdb->dfa->tables[YYTD_ID_ACCEPT2]->td_flags = tdflags;
791 }
792 /*
793 * Unfortunately due to a bug in earlier userspaces, a
794 * transition table may be present even when the dfa is
795 * not. For compatibility reasons unpack and discard.
796 */
797 if (!unpack_trans_table(e, &pdb->trans) && required_trans) {
798 *info = "failed to unpack profile transition table";
799 goto fail;
800 }
801
802 if (!pdb->dfa && pdb->trans.table)
803 aa_free_str_table(&pdb->trans);
804
805 /* TODO: move compat mapping here, requires dfa merging first */
806 /* TODO: move verify here, it has to be done after compat
mappings */
807 out:
808 *policy = pdb;
809 return 0;
810
811 fail:
812 aa_put_pdb(pdb);
813 e->pos = pos;
814 return error;
815 }
regards,
dan carpenter
