ZFS setgid broken on 0.7

Bug #1753288 reported by Stéphane Graber on 2018-03-04
10
This bug affects 1 person
Affects Status Importance Assigned to Milestone
linux (Ubuntu)
Critical
Colin Ian King
Bionic
Critical
Colin Ian King
zfs-linux (Ubuntu)
Critical
Colin Ian King
Bionic
Critical
Colin Ian King

Bug Description

Hey there,

We've had one of our LXD users report that setting the setgid bit inside a container using ZFS on Ubuntu 18.04 (zfs 0.7) is silently failing. This is not a LXD bug as the exact same operation works on other filesystems.

There are more details available here:
https://github.com/lxc/lxd/issues/4294

Reproducer looks something like:

```
root@c1:~# touch a
root@c1:~# chmod g+s a
root@c1:~# touch b
root@c1:~# chown 0:117 b
root@c1:~# chmod g+s b
root@c1:~# stat a
  File: a
  Size: 0 Blocks: 1 IO Block: 131072 regular empty file
Device: 43h/67d Inode: 33890 Links: 1
Access: (2644/-rw-r-Sr--) Uid: ( 0/ root) Gid: ( 0/ root)
Access: 2018-03-02 03:32:47.019430367 +0000
Modify: 2018-03-02 03:32:47.019430367 +0000
Change: 2018-03-02 03:32:49.459445015 +0000
 Birth: -
root@c1:~# stat b
  File: b
  Size: 0 Blocks: 1 IO Block: 131072 regular empty file
Device: 43h/67d Inode: 34186 Links: 1
Access: (0644/-rw-r--r--) Uid: ( 0/ root) Gid: ( 117/postdrop)
Access: 2018-03-02 03:32:50.907453706 +0000
Modify: 2018-03-02 03:32:50.907453706 +0000
Change: 2018-03-02 03:33:01.299516054 +0000
 Birth: -
root@c1:~#
```

And for confirmation, using a tmpfs in the same container:

```
root@c1:~# mkdir tmpfs
root@c1:~# mount -t tmpfs tmpfs tmpfs
root@c1:~# cd tmpfs/
root@c1:~/tmpfs# touch a
root@c1:~/tmpfs# chmod g+s a
root@c1:~/tmpfs# touch b
root@c1:~/tmpfs# chown 0:117 b
root@c1:~/tmpfs# chmod g+s b
root@c1:~/tmpfs# stat a
  File: a
  Size: 0 Blocks: 0 IO Block: 4096 regular empty file
Device: 65h/101d Inode: 3 Links: 1
Access: (2644/-rw-r-Sr--) Uid: ( 0/ root) Gid: ( 0/ root)
Access: 2018-03-02 03:33:35.783722623 +0000
Modify: 2018-03-02 03:33:35.783722623 +0000
Change: 2018-03-02 03:33:40.507750883 +0000
 Birth: -
root@c1:~/tmpfs# stat b
  File: b
  Size: 0 Blocks: 0 IO Block: 4096 regular empty file
Device: 65h/101d Inode: 4 Links: 1
Access: (2644/-rw-r-Sr--) Uid: ( 0/ root) Gid: ( 117/postdrop)
Access: 2018-03-02 03:33:42.131760597 +0000
Modify: 2018-03-02 03:33:42.131760597 +0000
Change: 2018-03-02 03:33:46.227785091 +0000
 Birth: -
root@c1:~/tmpfs#
```

This is particularly troubling because there are no errors returned to the user, so we now have containers that will have broken binaries and permissions applied to them with no visible way to detect the problem short of scanning the entire filesystem against a list of known permissions.

CVE References

Colin Ian King (colin-king) wrote :

Just checking, is this a regression in ZFS 0.7.x?

Changed in linux (Ubuntu):
assignee: nobody → Colin Ian King (colin-king)
Colin Ian King (colin-king) wrote :

If I'm not mistaken, this seems the same issue, correct me if I'm wrong: https://github.com/zfsonlinux/zfs/issues/6800

Stéphane Graber (stgraber) wrote :

That looks like it, yes. As far as I know most of us only noticed this when bionic switched from 0.6.x to 0.7.x so yes, 0.6.x seems fine and current 0.7.x is affected.

I've commented on the github issue and will reach out to Wolfgang (Blub) on IRC otherwise (he hangs out in the LXC/LXD dev channel) to see if he made any progress on this since November.

Changed in zfs-linux (Ubuntu):
importance: Undecided → Critical
Changed in linux (Ubuntu):
status: Triaged → In Progress
Changed in zfs-linux (Ubuntu):
status: New → In Progress
assignee: nobody → Colin Ian King (colin-king)
Seth Forshee (sforshee) on 2018-03-08
Changed in linux (Ubuntu):
status: In Progress → Fix Committed
Launchpad Janitor (janitor) wrote :

This bug was fixed in the package zfs-linux - 0.7.5-1ubuntu5

---------------
zfs-linux (0.7.5-1ubuntu5) bionic; urgency=medium

  * Fix ZFS setgid broken on 0.7 (LP: #1753288)
    Pull in upstream commit 0e85048f53e4, namely:
    "Take user namespaces into account in policy checks"
    - Change file related checks to use user namespaces and make
      sure involved uids/gids are mappable in the current
      namespace.

 -- Colin Ian King <email address hidden> Thu, 8 Mar 2018 09:10:00 +0000

Changed in zfs-linux (Ubuntu):
status: In Progress → Fix Released
Colin Ian King (colin-king) wrote :

Note: fix is dependent on kernel update

Launchpad Janitor (janitor) wrote :
Download full text (32.6 KiB)

This bug was fixed in the package linux - 4.15.0-13.14

---------------
linux (4.15.0-13.14) bionic; urgency=medium

  * linux: 4.15.0-13.14 -proposed tracker (LP: #1756408)

  * devpts: handle bind-mounts (LP: #1755857)
    - SAUCE: devpts: hoist out check for DEVPTS_SUPER_MAGIC
    - SAUCE: devpts: resolve devpts bind-mounts
    - SAUCE: devpts: comment devpts_mntget()
    - SAUCE: selftests: add devpts selftests

  * [bionic][arm64] d-i: add hisi_sas_v3_hw to scsi-modules (LP: #1756103)
    - d-i: add hisi_sas_v3_hw to scsi-modules

  * [Bionic][ARM64] enable ROCE and HNS3 driver support for hip08 SoC
    (LP: #1756097)
    - RDMA/hns: Refactor eq code for hip06
    - RDMA/hns: Add eq support of hip08
    - RDMA/hns: Add detailed comments for mb() call
    - RDMA/hns: Add rq inline data support for hip08 RoCE
    - RDMA/hns: Update the usage of sr_max and rr_max field
    - RDMA/hns: Set access flags of hip08 RoCE
    - RDMA/hns: Filter for zero length of sge in hip08 kernel mode
    - RDMA/hns: Fix QP state judgement before sending work requests
    - RDMA/hns: Assign dest_qp when deregistering mr
    - RDMA/hns: Fix endian problems around imm_data and rkey
    - RDMA/hns: Assign the correct value for tx_cqn
    - RDMA/hns: Create gsi qp in hip08
    - RDMA/hns: Add gsi qp support for modifying qp in hip08
    - RDMA/hns: Fill sq wqe context of ud type in hip08
    - RDMA/hns: Assign zero for pkey_index of wc in hip08
    - RDMA/hns: Update the verbs of polling for completion
    - RDMA/hns: Set the guid for hip08 RoCE device
    - net: hns3: Refactor of the reset interrupt handling logic
    - net: hns3: Add reset service task for handling reset requests
    - net: hns3: Refactors the requested reset & pending reset handling code
    - net: hns3: Add HNS3 VF IMP(Integrated Management Proc) cmd interface
    - net: hns3: Add mailbox support to VF driver
    - net: hns3: Add HNS3 VF HCL(Hardware Compatibility Layer) Support
    - net: hns3: Add HNS3 VF driver to kernel build framework
    - net: hns3: Unified HNS3 {VF|PF} Ethernet Driver for hip08 SoC
    - net: hns3: Add mailbox support to PF driver
    - net: hns3: Change PF to add ring-vect binding & resetQ to mailbox
    - net: hns3: Add mailbox interrupt handling to PF driver
    - net: hns3: add support to query tqps number
    - net: hns3: add support to modify tqps number
    - net: hns3: change the returned tqp number by ethtool -x
    - net: hns3: free the ring_data structrue when change tqps
    - net: hns3: get rss_size_max from configuration but not hardcode
    - net: hns3: add a mask initialization for mac_vlan table
    - net: hns3: add vlan offload config command
    - net: hns3: add ethtool related offload command
    - net: hns3: add handling vlan tag offload in bd
    - net: hns3: cleanup mac auto-negotiation state query
    - net: hns3: fix for getting auto-negotiation state in hclge_get_autoneg
    - net: hns3: add support for set_pauseparam
    - net: hns3: add support to update flow control settings after autoneg
    - net: hns3: add Asym Pause support to phy default features
    - net: hns3: add support for querying advertised pause frame by ethtool ethx
    - net:...

Changed in linux (Ubuntu Bionic):
status: Fix Committed → Fix Released
Brad Figg (brad-figg) wrote :

This bug is awaiting verification that the kernel in -proposed solves the problem. Please test the kernel and update this bug with the results. If the problem is solved, change the tag 'verification-needed-bionic' to 'verification-done-bionic'. If the problem still exists, change the tag 'verification-needed-bionic' to 'verification-failed-bionic'.

If verification is not done by 5 working days from today, this fix will be dropped from the source code, and this bug will be closed.

See https://wiki.ubuntu.com/Testing/EnableProposed for documentation how to enable and use -proposed. Thank you!

tags: added: verification-needed-bionic
Andy Whitcroft (apw) on 2019-02-14
tags: added: kernel-fixup-verification-needed-bionic
removed: verification-needed-bionic
Andy Whitcroft (apw) wrote :

This bug was erroneously marked for verification in bionic; verification is not required and verification-needed-bionic is being removed.

tags: added: verification-done-bionic
To post a comment you must log in.
This report contains Public information  Edit
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.