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

On 11/19/20 3:54 PM, Jin Yan wrote: > 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> This is what I have in XML: <serial type='pipe'> <source path='/tmp/test_pipe'/> <target type='isa-serial' port='0'> <model name='isa-serial'/> </target> </serial> and the file doesn't exist. > 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 ... > I don't get a warning. I get a regular error: error: unable to set security context 'unconfined_u:object_r:svirt_image_t:s0:c339,c673' on '/tmp/test_pipe': No such file or directory

> 2. The next secdriver about setting DAC-label run in > virSecurityDACTransactionRun() return false because above file does not > exist. > Oh I think get it now. So if one secdriver would fail but not the other then because of transactions the rollback would be ineffective?

> 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? I guess that is the source of problem. Have you tried the latest release (or master)?