• Igor Zhbanov's avatar
    Fix NULL pointer dereference in smack_inode_unlink() and smack_inode_rmdir() · cdb56b60
    Igor Zhbanov authored
    This patch fixes kernel Oops because of wrong common_audit_data type
    in smack_inode_unlink() and smack_inode_rmdir().
    
    When SMACK security module is enabled and SMACK logging is on (/smack/logging
    is not zero) and you try to delete the file which
    1) you cannot delete due to SMACK rules and logging of failures is on
    or
    2) you can delete and logging of success is on,
    
    you will see following:
    
    	Unable to handle kernel NULL pointer dereference at virtual address 000002d7
    
    	[<...>] (strlen+0x0/0x28)
    	[<...>] (audit_log_untrustedstring+0x14/0x28)
    	[<...>] (common_lsm_audit+0x108/0x6ac)
    	[<...>] (smack_log+0xc4/0xe4)
    	[<...>] (smk_curacc+0x80/0x10c)
    	[<...>] (smack_inode_unlink+0x74/0x80)
    	[<...>] (security_inode_unlink+0x2c/0x30)
    	[<...>] (vfs_unlink+0x7c/0x100)
    	[<...>] (do_unlinkat+0x144/0x16c)
    
    The function smack_inode_unlink() (and smack_inode_rmdir()) need
    to log two structures of different types. First of all it does:
    
    	smk_ad_init(&ad, __func__, LSM_AUDIT_DATA_DENTRY);
    	smk_ad_setfield_u_fs_path_dentry(&ad, dentry);
    
    This will set common audit data type to LSM_AUDIT_DATA_DENTRY
    and store dentry for auditing (by function smk_curacc(), which in turn calls
    dump_common_audit_data(), which is actually uses provided data and logs it).
    
    	/*
    	 * You need write access to the thing you're unlinking
    	 */
    	rc = smk_curacc(smk_of_inode(ip), MAY_WRITE, &ad);
    	if (rc == 0) {
    		/*
    		 * You also need write access to the containing directory
    		 */
    
    Then this function wants to log anoter data:
    
    		smk_ad_setfield_u_fs_path_dentry(&ad, NULL);
    		smk_ad_setfield_u_fs_inode(&ad, dir);
    
    The function sets inode field, but don't change common_audit_data type.
    
    		rc = smk_curacc(smk_of_inode(dir), MAY_WRITE, &ad);
    	}
    
    So the dump_common_audit() function incorrectly interprets inode structure
    as dentry, and Oops will happen.
    
    This patch reinitializes common_audit_data structures with correct type.
    Also I removed unneeded
    	smk_ad_setfield_u_fs_path_dentry(&ad, NULL);
    initialization, because both dentry and inode pointers are stored
    in the same union.
    Signed-off-by: default avatarIgor Zhbanov <i.zhbanov@samsung.com>
    Signed-off-by: default avatarKyungmin Park <kyungmin.park@samsung.com>
    cdb56b60
smack_lsm.c 84.1 KB