Bugzilla will be upgraded to version 5.0. The upgrade date is tentatively scheduled for 2 December 2018, pending final testing and feedback.
Bug 889255 - Monitor command acl_remove messes up the ACL
Monitor command acl_remove messes up the ACL
Status: CLOSED ERRATA
Product: Red Hat Enterprise Linux 6
Classification: Red Hat
Component: qemu-kvm (Show other bugs)
6.4
Unspecified Unspecified
unspecified Severity unspecified
: rc
: ---
Assigned To: Markus Armbruster
Virtualization Bugs
:
Depends On:
Blocks:
  Show dependency treegraph
 
Reported: 2012-12-20 10:48 EST by Markus Armbruster
Modified: 2013-11-21 01:27 EST (History)
13 users (show)

See Also:
Fixed In Version: qemu-kvm-0.12.1.2-2.390.el6
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2013-11-21 01:27:12 EST
Type: Bug
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---


Attachments (Terms of Use)


External Trackers
Tracker ID Priority Status Summary Last Updated
Red Hat Product Errata RHSA-2013:1553 normal SHIPPED_LIVE Important: qemu-kvm security, bug fix, and enhancement update 2013-11-20 16:40:29 EST

  None (edit)
Description Markus Armbruster 2012-12-20 10:48:58 EST
Description of problem:
qemu_acl_remove() fails to decrement acl->nentries, leaving the ACL in an inconsistent state.  This can make future acl_add misreport the position, or silently fail to add anything.

Version-Release number of selected component (if applicable):
qemu-kvm-0.12.1.2-2.344.el6

How reproducible:
100%

Steps to Reproduce:
1. Start qemu-kvm in a way that creates an ACL:
$ qemu-kvm -nodefaults -S -vnc :0,acl,sasl -monitor stdio
2. Add a few entries in the monitor:
(qemu) acl_add vnc.username eins allow
(qemu) acl_add vnc.username zwei allow
(qemu) acl_add vnc.username drei allow
(qemu) acl_add vnc.username vier allow
(qemu) acl_show vnc.username
3. Delete a few:
(qemu) acl_remove vnc.username vier
(qemu) acl_remove vnc.username drei
(qemu) acl_show vnc.username
4. Add some more:
acl_add vnc.username lost allow 3
acl_show vnc.username
acl_add vnc.username wrongpos allow
acl_show vnc.username

Actual results:
(qemu) acl_add vnc.username eins allow
acl: added rule at position 1
(qemu) acl_add vnc.username zwei allow
acl: added rule at position 2
(qemu) acl_add vnc.username drei allow
acl: added rule at position 3
(qemu) acl_add vnc.username vier allow
acl: added rule at position 4
(qemu) acl_show vnc.username
policy: deny
1: allow eins
2: allow zwei
3: allow drei
4: allow vier
(qemu) acl_remove vnc.username vier
acl: removed rule at position 4
(qemu) acl_remove vnc.username drei
acl: removed rule at position 3
(qemu) acl_show vnc.username
policy: deny
1: allow eins
2: allow zwei
(qemu) acl_add vnc.username lost allow 3
acl: added rule at position 2
(qemu) acl_show vnc.username
policy: deny
1: allow eins
2: allow zwei
(qemu) acl_add vnc.username wrongpos allow
acl: added rule at position 5
(qemu) acl_show vnc.username
policy: deny
1: allow eins
2: allow zwei
3: allow wrongpos
(qemu) q

Note that both of step 4's acl_add report bogus position, and the first one doesn't add anything.

Expected results:
Step 4's acl_add both add a rule and report the correct position.
Comment 1 Markus Armbruster 2012-12-20 10:53:04 EST
libvirt doesn't use acl_remove as far as I can tell.
Comment 3 Markus Armbruster 2013-01-16 05:36:16 EST
Fixed upstream in commit c23c15d acl: Fix acl_remove not to mess up the ACL
Comment 5 Chao Yang 2013-06-04 03:20:22 EDT
Reproduced this issue with qemu-kvm-0.12.1.2-2.375.el6.x86_64, 2.6.32-383.el6.x86_64. Same steps as reproducer in Comment #0. acl_show reported incorrect position. 

Steps:
1. add three rules by:
(qemu) acl_add vnc.username aaa allow
acl: added rule at position 1
(qemu) acl_add vnc.username bbb allow
acl: added rule at position 2
(qemu) acl_add vnc.username ccc allow
acl: added rule at position 3
(qemu) acl_show vnc.username
policy: deny
1: allow aaa
2: allow bbb
3: allow ccc

2. remove two rules by:
(qemu) acl_remove vnc.username ccc
acl: removed rule at position 3
(qemu) acl_remove vnc.username bbb
acl: removed rule at position 2
(qemu) acl_show vnc.username
policy: deny
1: allow aaa

3. add another two rules:
(qemu) acl_add vnc.username ddd allow 2
acl: added rule at position 1
(qemu) acl_show vnc.username
policy: deny
1: allow aaa
(qemu) acl_add vnc.username eee allow 
acl: added rule at position 4
(qemu) acl_show vnc.username
policy: deny
1: allow aaa
2: allow eee

Providing with qa_ack+ based on above.
Comment 6 Chao Yang 2013-06-04 03:23:30 EDT
CLI:
/usr/libexec/qemu-kvm -M rhel6.5.0 -cpu Nehalem -enable-kvm -m 4096 -smp 4,sockets=2,cores=2,threads=1 -name test -rtc base=utc -nodefaults -drive file=/home/images/rhel6.4.qcow2,if=none,id=drive-virtio-disk0,format=qcow2,cache=none,werror=stop,rerror=stop,aio=native -device virtio-blk-pci,scsi=off,bus=pci.0,addr=0x5,drive=drive-virtio-disk0,id=virtio-disk0,bootindex=1 -net none -k en-us -vga cirrus -vnc :1,acl,sasl -monitor stdio -S
Comment 7 Chao Yang 2013-06-04 03:32:48 EDT
Hi Markus,
 I have a question about acl_add. From the output of help acl_add below:
(qemu) help acl_add
acl_add aclname match allow|deny [index] -- add a match rule to the access control list

If 'index' above means the position, I ran: '(qemu) acl_add vnc.username ddd allow 2 ', shouldn't it report position 1? I didn't remove any rule, this was a new qemu-kvm instance.

(qemu) acl_show vnc.username
policy: deny
(qemu) acl_add vnc.username ddd allow 2
acl: added rule at position 1
(qemu) acl_show vnc.username
policy: deny
1: allow ddd
Comment 8 Markus Armbruster 2013-06-04 04:38:48 EDT
Three cases for index:

1. index <= 0: error
2. 0 < index < length of list: insert before the i-th element of the list (counting from 1)
3. index >= length of list: append to list

Your acl_add is case 3.  It tells you the position actually used: added rule at position 1.  Looks fine to me.

However, there's another bug: inserting before the last element of the list is broken!  That's because index == length is case 3 instead of 2.  Bug 970516.
Comment 9 Chao Yang 2013-06-04 05:29:00 EDT
(In reply to Markus Armbruster from comment #8)
> Three cases for index:
> 
> 1. index <= 0: error
> 2. 0 < index < length of list: insert before the i-th element of the list
> (counting from 1)
> 3. index >= length of list: append to list
> 
> Your acl_add is case 3.  It tells you the position actually used: added rule
> at position 1.  Looks fine to me.
> 
> However, there's another bug: inserting before the last element of the list
> is broken!  That's because index == length is case 3 instead of 2.  Bug
> 970516.

Thanks very much for your inputs, they are helpful for QE to add such tests.
Comment 18 mazhang 2013-09-09 06:07:21 EDT
Reproduce this bug on qemu-kvm-0.12.1.2-2.376.el6.x86_64.

[root@localhost qemu-kvm-376]# rpm -qa |grep qemu
gpxe-roms-qemu-0.9.7-6.10.el6.noarch
qemu-kvm-tools-0.12.1.2-2.376.el6.x86_64
qemu-kvm-0.12.1.2-2.376.el6.x86_64
qemu-kvm-debuginfo-0.12.1.2-2.376.el6.x86_64
qemu-img-0.12.1.2-2.376.el6.x86_64

Steps to Reproduce:
1. Start qemu-kvm in a way that creates an ACL:
$ /usr/libexec/qemu-kvm -nodefaults -S -vnc :0,acl,sasl -monitor stdio
2. Add a few entries in the monitor:
(qemu) acl_add vnc.username eins allow
(qemu) acl_add vnc.username zwei allow
(qemu) acl_add vnc.username drei allow
(qemu) acl_add vnc.username vier allow
(qemu) acl_show vnc.username
3. Delete a few:
(qemu) acl_remove vnc.username vier
(qemu) acl_remove vnc.username drei
(qemu) acl_show vnc.username
4. Add some more:
acl_add vnc.username lost allow 3
acl_show vnc.username
acl_add vnc.username wrongpos allow
acl_show vnc.username

Actual results:
[root@localhost ~]# /usr/libexec/qemu-kvm -nodefaults -S -vnc :0,acl,sasl -monitor stdio
QEMU 0.12.1 monitor - type 'help' for more information
(qemu) acl_add vnc.username eins allow
acl: added rule at position 1
(qemu) acl_add vnc.username zwei allow
acl: added rule at position 2
(qemu) acl_add vnc.username drei allow
acl: added rule at position 3
(qemu) acl_add vnc.username vier allow
acl: added rule at position 4
(qemu) acl_show vnc.username
policy: deny
1: allow eins
2: allow zwei
3: allow drei
4: allow vier
(qemu) acl_remove vnc.username vier
acl: removed rule at position 4
(qemu) acl_remove vnc.username drei
acl: removed rule at position 3
(qemu) acl_show vnc.username
policy: deny
1: allow eins
2: allow zwei
(qemu) acl_add vnc.username lost allow 3
acl: added rule at position 2
(qemu) acl_show vnc.username
policy: deny
1: allow eins
2: allow zwei
(qemu) acl_add vnc.username wrongpos allow
acl: added rule at position 5
(qemu) acl_show vnc.username
policy: deny
1: allow eins
2: allow zwei
3: allow wrongpos

Expected results:
Step 4's acl_add both add a rule and report the correct position.
Comment 19 mazhang 2013-09-09 07:19:51 EDT
Verify this bug on qemu-kvm-0.12.1.2-2.400.el6.x86_64 

[root@localhost ~]# rpm -qa |grep qemu
gpxe-roms-qemu-0.9.7-6.10.el6.noarch
qemu-kvm-tools-0.12.1.2-2.400.el6.x86_64
qemu-kvm-0.12.1.2-2.400.el6.x86_64
qemu-kvm-debuginfo-0.12.1.2-2.400.el6.x86_64
qemu-img-0.12.1.2-2.400.el6.x86_64

Steps:
1. Start qemu-kvm in a way that creates an ACL:
$ /usr/libexec/qemu-kvm -nodefaults -S -vnc :0,acl,sasl -monitor stdio
2. Add a few entries in the monitor:
(qemu) acl_add vnc.username eins allow
(qemu) acl_add vnc.username zwei allow
(qemu) acl_add vnc.username drei allow
(qemu) acl_add vnc.username vier allow
(qemu) acl_show vnc.username
3. Delete a few:
(qemu) acl_remove vnc.username vier
(qemu) acl_remove vnc.username drei
(qemu) acl_show vnc.username
4. Add some more:
acl_add vnc.username lost allow 3
acl_show vnc.username
acl_add vnc.username wrongpos allow
acl_show vnc.username

Actual results:
[root@localhost ~]# /usr/libexec/qemu-kvm -nodefaults -S -vnc :0,acl,sasl -monitor stdio
QEMU 0.12.1 monitor - type 'help' for more information
(qemu) acl_add vnc.username eins allow
acl: added rule at position 1
(qemu) acl_add vnc.username zwei allow
acl: added rule at position 2
(qemu) acl_add vnc.username drei allow
acl: added rule at position 3
(qemu) acl_add vnc.username vier allow
acl: added rule at position 4
(qemu) acl_show vnc.username
policy: deny
1: allow eins
2: allow zwei
3: allow drei
4: allow vier
(qemu) acl_remove vnc.username vier
acl: removed rule at position 4
(qemu) acl_remove vnc.username drei
acl: removed rule at position 3
(qemu) acl_show vnc.username
policy: deny
1: allow eins
2: allow zwei
(qemu) acl_add vnc.username lost allow 3
acl: added rule at position 3
(qemu) acl_show vnc.username
policy: deny
1: allow eins
2: allow zwei
3: allow lost
(qemu) acl_add vnc.username wrongpos allow
acl: added rule at position 4
(qemu) acl_show vnc.username
policy: deny
1: allow eins
2: allow zwei
3: allow lost
4: allow wrongpos

Step 4 acl_add both add a rule and report the correct position.

So this bug has been fixed.
Comment 21 errata-xmlrpc 2013-11-21 01:27:12 EST
Since the problem described in this bug report should be
resolved in a recent advisory, it has been closed with a
resolution of ERRATA.

For information on the advisory, and where to find the updated
files, follow the link below.

If the solution does not work for you, open a new bug report.

http://rhn.redhat.com/errata/RHSA-2013-1553.html

Note You need to log in before you can comment on or make changes to this bug.