[PATCH v2] selinux label: restore all labels when some labels fail to set

[Jin Yan]

On 2020/11/13 22:33, Michal Privoznik wrote:
> On 11/13/20 10:47 AM, Jin Yan wrote:
>
>> Hi Michal,
>> I found this problem while performing migration, based on
>>       libvirt version: 6.2.0
>>       SELinux mode: permissive
>>
>> Steps:
>> 1. start a vm configured with pipe-type serial port.
>>       <serial type='pipe'>
>>         <source path='/tmp/test_pipe'/>
>>         <target type='system-serial' port='1'>
>>           <model name='pl011'/>
>>         </target>
>>       </serial>
>> 2. migrate vm to Dst-side where no '/tmp/test_pipe' exits.
>> 3. migration failed in Dst-side qemuProcessLaunch, and the path's label
>> that
>> has been set is not restored ('/var/lib/libvirt/qemu/nvram/XXX.fd').
>>
>> I have no idea why 2)rollback you mentioned didn't work.
>>
>>
> I'm not sure. I could not reproduce with the current master. Is it
> possible for you to try the master?
>
> Michal

I think we can reproduce it in a more easier way, that is, starting a VM whose XML is configured with a pipe file that does not exist on local host:
	<serial type='pipe'>
        <source path='/tmp/serial.pipe'/>
        <target port='0'/>
      </serial>

1. Though '/tmp/serial.pipe' does not exist, this secdriver (if I'm not mistaken about this concept) set SELinux-label return success, and the marked items (eg. XXX.fd, XXX.iso) will not be rollback.

[call trace]:
virSecuritySELinuxTransactionRun	   -- return 0
     virSecuritySELinuxSetFilecon           -- return 0
         virSecuritySELinuxSetFileconImpl   -- return 1, warned unable to ...

2. The next secdriver about setting DAC-label run in virSecurityDACTransactionRun() return false because above file does not exist.

virSecurityManagerTransactionCommit() return false, but where is the rollback performed for other secdrivers (here means setting SELinux-label in 1) ? I don't quite understand the second point you mentioned in your last reply:
     ---
     2) rollback for other secdrivers after one failed is handled in virSecurityStackSetAllLabel().
     ---

In addition, is there any wrong in virSecuritySELinuxTransactionRun return success while '/tmp/serial.pipe' does not exist?

Yan

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20201119/040b8228/attachment-0001.htm>