From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ernesto =?utf-8?Q?A=2E_Fern=C3=A1ndez?= Subject: [PATCH 0/5] Failure to set acl may alter group permissions Date: Wed, 12 Jul 2017 06:53:22 -0300 Message-ID: Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit To: Jan Kara , Theodore Ts'o , Andreas Dilger , Dave Kleikamp , linux-ext4@vger.kernel.org, jfs-discussion@lists.sourceforge.net, reiserfs-devel@vger.kernel.org Return-path: Content-Disposition: inline Sender: reiserfs-devel-owner@vger.kernel.org List-Id: linux-ext4.vger.kernel.org When changing a file's acl mask, the function that sets the access control list (ext2_set_acl(), __ext4_set_acl(), __jfs_set_acl()...) will first set the group permission bits of the file to the value of the mask (by calling posix_acl_update_mode()), and only then set the actual extended attribute representing the new acl. The problem is, none of these functions try to restore the original permission bits if the second part fails. If this happens to a file that had no acl attributes to begin with, the system will from now on assume that the mask permission bits are actual group permission bits, potentially granting access to the wrong users. If your working directory is on a filesystem mounted with extended user attributes (and acl of course), this script will trigger the issue by filling the drive: touch test.file chmod go-rwx test.file yes xxxxxxxxxx > test.file i=1 while setfattr -n user.$i test.file; do ((++i)) done setfacl -m m:r test.file By the time the script returns, the group that owns test.file may have read permissions that were never granted. This happens reliably on at least ext2, ext4, jfs and reiserfs. I will follow this mail with patch drafts for those filesystems. I believe most filesystems that support acl will need a patch, but perhaps it's best if I share what I have so far. Thank you for your attention. Ernesto A. Fernández (5): ext4: preserve i_mode if __ext4_set_acl() fails ext2: preserve i_mode if ext2_set_acl() fails ext2: fix line over 80 characters in ext2_set_acl() jfs: preserve i_mode if __jfs_set_acl() fails reiserfs: preserve i_mode if __reiserfs_set_acl() fails fs/ext2/acl.c | 25 ++++++++++++++++--------- fs/ext4/acl.c | 15 +++++++++++---- fs/jfs/acl.c | 15 +++++++++++---- fs/reiserfs/xattr.c | 4 ++++ fs/reiserfs/xattr_acl.c | 25 +++++++++---------------- 5 files changed, 51 insertions(+), 33 deletions(-) -- 2.1.4 From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ernesto =?utf-8?Q?A=2E_Fern=C3=A1ndez?= Subject: [PATCH 0/5] Failure to set acl may alter group permissions Date: Wed, 12 Jul 2017 06:53:22 -0300 Message-ID: Mime-Version: 1.0 Content-Transfer-Encoding: base64 Return-path: DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=date:from:to:subject:message-id:mime-version:content-disposition :content-transfer-encoding; bh=8k/dGCjHWxxQsGx1SduWsInZm6IbTt0px4jFIEvvJ8I=; b=eGXZ7LbCf1z/AHyH2YVD+W49II8qetL2vZLhQkY2Ud+zhXs1D+fCppDy01sLuHaENT Mjt9246ShWlNjXhzdF1psbDEqyucF3gs4OdbekjZ/so88Bngbv93jtPnGKV1THvlKRdH NOaY6ngd/1jN5sBfNXVvpYPVRrJleCZhUifK8RZBNbJcbupZ712Rz9oWGd/UJVSpCAjt i02dBhiUHy2BpBYw4bvi+SLZwZ3FeU+EjxYdHXnJOR93y5f9DyTDAz5icy1JR3FUL5vc VCoMeoRg7sHGeuLWOOPZ14LjDecczf5WF9Sbfwn5ygE4wmrdxM/bExM1S+rcAyxzgZl4 K21A== Content-Disposition: inline Sender: reiserfs-devel-owner@vger.kernel.org List-ID: Content-Type: text/plain; charset="macroman" To: Jan Kara , Theodore Ts'o , Andreas Dilger , Dave Kleikamp , linux-ext4@vger.kernel.org, jfs-discussion@lists.sourceforge.net, reiserfs-devel@vger.kernel.org V2hlbiBjaGFuZ2luZyBhIGZpbGUncyBhY2wgbWFzaywgdGhlIGZ1bmN0aW9uIHRoYXQgc2V0cyB0 aGUgYWNjZXNzIGNvbnRyb2wKbGlzdCAoZXh0Ml9zZXRfYWNsKCksIF9fZXh0NF9zZXRfYWNsKCks IF9famZzX3NldF9hY2woKS4uLikgd2lsbCBmaXJzdCBzZXQKdGhlIGdyb3VwIHBlcm1pc3Npb24g Yml0cyBvZiB0aGUgZmlsZSB0byB0aGUgdmFsdWUgb2YgdGhlIG1hc2sgKGJ5IGNhbGxpbmcKcG9z aXhfYWNsX3VwZGF0ZV9tb2RlKCkpLCBhbmQgb25seSB0aGVuIHNldCB0aGUgYWN0dWFsIGV4dGVu ZGVkIGF0dHJpYnV0ZQpyZXByZXNlbnRpbmcgdGhlIG5ldyBhY2wuCgpUaGUgcHJvYmxlbSBpcywg bm9uZSBvZiB0aGVzZSBmdW5jdGlvbnMgdHJ5IHRvIHJlc3RvcmUgdGhlIG9yaWdpbmFsCnBlcm1p c3Npb24gYml0cyBpZiB0aGUgc2Vjb25kIHBhcnQgZmFpbHMuIElmIHRoaXMgaGFwcGVucyB0byBh IGZpbGUgdGhhdApoYWQgbm8gYWNsIGF0dHJpYnV0ZXMgdG8gYmVnaW4gd2l0aCwgdGhlIHN5c3Rl bSB3aWxsIGZyb20gbm93IG9uIGFzc3VtZQp0aGF0IHRoZSBtYXNrIHBlcm1pc3Npb24gYml0cyBh cmUgYWN0dWFsIGdyb3VwIHBlcm1pc3Npb24gYml0cywKcG90ZW50aWFsbHkgZ3JhbnRpbmcgYWNj ZXNzIHRvIHRoZSB3cm9uZyB1c2Vycy4KCklmIHlvdXIgd29ya2luZyBkaXJlY3RvcnkgaXMgb24g YSBmaWxlc3lzdGVtIG1vdW50ZWQgd2l0aCBleHRlbmRlZCB1c2VyCmF0dHJpYnV0ZXMgKGFuZCBh Y2wgb2YgY291cnNlKSwgdGhpcyBzY3JpcHQgd2lsbCB0cmlnZ2VyIHRoZSBpc3N1ZSBieQpmaWxs aW5nIHRoZSBkcml2ZToKCgp0b3VjaCB0ZXN0LmZpbGUKY2htb2QgZ28tcnd4IHRlc3QuZmlsZQp5 ZXMgeHh4eHh4eHh4eCA+IHRlc3QuZmlsZQppPTEKd2hpbGUgc2V0ZmF0dHIgLW4gdXNlci4kaSB0 ZXN0LmZpbGU7IGRvCiAgKCgrK2kpKQpkb25lCnNldGZhY2wgLW0gbTpyIHRlc3QuZmlsZQoKCkJ5 IHRoZSB0aW1lIHRoZSBzY3JpcHQgcmV0dXJucywgdGhlIGdyb3VwIHRoYXQgb3ducyB0ZXN0LmZp bGUgbWF5IGhhdmUgcmVhZApwZXJtaXNzaW9ucyB0aGF0IHdlcmUgbmV2ZXIgZ3JhbnRlZC4gVGhp cyBoYXBwZW5zIHJlbGlhYmx5IG9uIGF0IGxlYXN0CmV4dDIsIGV4dDQsIGpmcyBhbmQgcmVpc2Vy ZnMuCgpJIHdpbGwgZm9sbG93IHRoaXMgbWFpbCB3aXRoIHBhdGNoIGRyYWZ0cyBmb3IgdGhvc2Ug ZmlsZXN5c3RlbXMuIEkgYmVsaWV2ZQptb3N0IGZpbGVzeXN0ZW1zIHRoYXQgc3VwcG9ydCBhY2wg d2lsbCBuZWVkIGEgcGF0Y2gsIGJ1dCBwZXJoYXBzIGl0J3MgYmVzdAppZiBJIHNoYXJlIHdoYXQg SSBoYXZlIHNvIGZhci4KClRoYW5rIHlvdSBmb3IgeW91ciBhdHRlbnRpb24uCgpFcm5lc3RvIEEu IEZlcm7DoW5kZXogKDUpOgogIGV4dDQ6IHByZXNlcnZlIGlfbW9kZSBpZiBfX2V4dDRfc2V0X2Fj bCgpIGZhaWxzCiAgZXh0MjogcHJlc2VydmUgaV9tb2RlIGlmIGV4dDJfc2V0X2FjbCgpIGZhaWxz CiAgZXh0MjogZml4IGxpbmUgb3ZlciA4MCBjaGFyYWN0ZXJzIGluIGV4dDJfc2V0X2FjbCgpCiAg amZzOiBwcmVzZXJ2ZSBpX21vZGUgaWYgX19qZnNfc2V0X2FjbCgpIGZhaWxzCiAgcmVpc2VyZnM6 IHByZXNlcnZlIGlfbW9kZSBpZiBfX3JlaXNlcmZzX3NldF9hY2woKSBmYWlscwoKIGZzL2V4dDIv YWNsLmMgICAgICAgICAgIHwgMjUgKysrKysrKysrKysrKysrKy0tLS0tLS0tLQogZnMvZXh0NC9h Y2wuYyAgICAgICAgICAgfCAxNSArKysrKysrKysrKy0tLS0KIGZzL2pmcy9hY2wuYyAgICAgICAg ICAgIHwgMTUgKysrKysrKysrKystLS0tCiBmcy9yZWlzZXJmcy94YXR0ci5jICAgICB8ICA0ICsr KysKIGZzL3JlaXNlcmZzL3hhdHRyX2FjbC5jIHwgMjUgKysrKysrKysrLS0tLS0tLS0tLS0tLS0t LQogNSBmaWxlcyBjaGFuZ2VkLCA1MSBpbnNlcnRpb25zKCspLCAzMyBkZWxldGlvbnMoLSkKCi0t IAoyLjEuNAoKLS0KVG8gdW5zdWJzY3JpYmUgZnJvbSB0aGlzIGxpc3Q6IHNlbmQgdGhlIGxpbmUg InVuc3Vic2NyaWJlIHJlaXNlcmZzLWRldmVsIiBpbgp0aGUgYm9keSBvZiBhIG1lc3NhZ2UgdG8g bWFqb3Jkb21vQHZnZXIua2VybmVsLm9yZwpNb3JlIG1ham9yZG9tbyBpbmZvIGF0ICBodHRwOi8v dmdlci5rZXJuZWwub3JnL21ham9yZG9tby1pbmZvLmh0bWwK