overlay fs regression: chmod fails with "Operation not permitted" on chowned files

Bug #1555997 reported by Martin Pitt
6
This bug affects 1 person
Affects Status Importance Assigned to Milestone
linux (Ubuntu)
Fix Released
Medium
Seth Forshee
Wily
Fix Released
Medium
Seth Forshee
Xenial
Fix Released
Medium
Seth Forshee

Bug Description

This is a regression in Xenial's kernel 4.4.0-9 or 4.4.0-10. See comment #3 for simple reproducer.

ORIGINAL BUG REPORT
===================
I'm investigating some failures in autopkgtest's testsuite, and stumbled over something really weird: In an ephemeral container it is apparently not possible any more to chmod files that started out being root owned and got chowned later:

$ sudo lxc-start-ephemeral -o adt-wily
(log in as ubuntu/ubuntu)
ubuntu@adt-wily-hvzj1eoa:~$ echo hello | sudo tee /tmp/testfile
[sudo] password for ubuntu:
hello
ubuntu@adt-wily-hvzj1eoa:~$ sudo chown ubuntu:ubuntu /tmp/testfile
ubuntu@adt-wily-hvzj1eoa:~$ chmod +x /tmp/testfile
chmod: changing permissions of ‘/tmp/testfile’: Operation not permitted

However, if the file was *not* previously chowned, it works as expected:

ubuntu@adt-wily-hvzj1eoa:~$ echo hello > /tmp/testfile2
ubuntu@adt-wily-hvzj1eoa:~$ chmod +x /tmp/testfile2
ubuntu@adt-wily-hvzj1eoa:~$ chmod -x /tmp/testfile2

(no errors and testfile2 becomes executable)

There is no visible permission difference in the files at all, other than being group-writable (but changing the group w bit in either direction does not change the error at all):

-rw-r--r-- 1 ubuntu ubuntu 6 Mar 11 10:26 /tmp/testfile
-rw-rw-r-- 1 ubuntu ubuntu 6 Mar 11 10:26 /tmp/testfile2

ubuntu@adt-wily-hvzj1eoa:~$ stat /tmp/testfile*
  File: ‘/tmp/testfile’
  Size: 6 Blocks: 8 IO Block: 4096 regular file
Device: 15h/21d Inode: 28 Links: 1
Access: (0644/-rw-r--r--) Uid: ( 1000/ ubuntu) Gid: ( 1000/ ubuntu)
Access: 2016-03-11 10:26:19.574364117 +0100
Modify: 2016-03-11 10:26:19.574364117 +0100
Change: 2016-03-11 10:26:21.930343210 +0100
 Birth: -
  File: ‘/tmp/testfile2’
  Size: 6 Blocks: 8 IO Block: 4096 regular file
Device: 15h/21d Inode: 29 Links: 1
Access: (0664/-rw-rw-r--) Uid: ( 1000/ ubuntu) Gid: ( 1000/ ubuntu)
Access: 2016-03-11 10:26:58.730145919 +0100
Modify: 2016-03-11 10:26:58.730145919 +0100
Change: 2016-03-11 10:27:44.530203985 +0100
 Birth: -

There are also no ACLs involved (I checked with getfacl).

This does not happen with a normal lxc-start, so this might very well be a bug in Linux' overlayfs. However, it also does not happen with the more modern "sudo lxc-copy -n adt-wily --ephemeral --foreground" -- bug perhaps this isn't using overlayfs?

ProblemType: Bug
DistroRelease: Ubuntu 16.04
Package: lxc 2.0.0~rc9-0ubuntu1
ProcVersionSignature: Ubuntu 4.4.0-11.26-generic 4.4.4
Uname: Linux 4.4.0-11-generic x86_64
ApportVersion: 2.20-0ubuntu3
Architecture: amd64
CurrentDesktop: i3
Date: Fri Mar 11 10:21:20 2016
EcryptfsInUse: Yes
PackageArchitecture: all
SourcePackage: lxc
UpgradeStatus: No upgrade log present (probably fresh install)
defaults.conf:
 lxc.network.type = veth
 lxc.network.link = lxcbr0
 lxc.network.flags = up
 lxc.network.hwaddr = 00:16:3e:xx:xx:xx
dnsmasq.conf:
 enable-tftp
 tftp-root=/tmp/tftp
 dhcp-boot=pxelinux.0
lxc.conf: lxc.lxcpath = /srv/lxc

Revision history for this message
Martin Pitt (pitti) wrote :
description: updated
description: updated
Revision history for this message
Martin Pitt (pitti) wrote :

I tried to purge all lxc/lxd packages and install LXC 2.0.0~rc1-0ubuntu1 again, but this now not working at all any more on xenial's kernel -- the container never starts and I get a lot of apparmor violations.

Revision history for this message
Martin Pitt (pitti) wrote :

This still works fine with Linux 4.4.0-8, but fails on 4.4.0-10, so it's a kernel regression.

This can be reproduced much simpler without LXC:

$ mkdir /tmp/lower /tmp/upper /tmp/work /tmp/merged
$ sudo mount -t overlay overlay -olowerdir=/tmp/lower,upperdir=/tmp/upper,workdir=/tmp/work /tmp/merged
$ sudo touch /tmp/merged/testfile
$ sudo chown $USER:$USER /tmp/merged/testfile
$ $ chmod +x /tmp/merged/testfile
chmod: changing permissions of '/tmp/merged/testfile': Operation not permitted

summary: - chmod fails with "Operation not permitted" on chowned files in ephemeral
- container
+ overlay fs: chmod fails with "Operation not permitted" on chowned files
summary: - overlay fs: chmod fails with "Operation not permitted" on chowned files
+ overlay fs regression: chmod fails with "Operation not permitted" on
+ chowned files
affects: lxc (Ubuntu) → linux (Ubuntu)
tags: added: regression-release
description: updated
Revision history for this message
Brad Figg (brad-figg) wrote : Missing required logs.

This bug is missing log files that will aid in diagnosing the problem. From a terminal window please run:

apport-collect 1555997

and then change the status of the bug to 'Confirmed'.

If, due to the nature of the issue you have encountered, you are unable to run this command, please add a comment stating that fact and change the bug status to 'Confirmed'.

This change has been made by an automated script, maintained by the Ubuntu Kernel Team.

Changed in linux (Ubuntu):
status: New → Incomplete
Revision history for this message
Martin Pitt (pitti) wrote :

Reproducer is available, not hw specific, so doesn’t need additional logs.

Changed in linux (Ubuntu):
status: Incomplete → Confirmed
tags: added: bot-stop-nagging
Changed in linux (Ubuntu):
importance: Undecided → Medium
tags: added: performing-bisect
Changed in linux (Ubuntu Xenial):
status: Confirmed → Triaged
Revision history for this message
Joseph Salisbury (jsalisbury) wrote :

Overlayfs commits between -8 and -10:

84005e9 UBUNTU: SAUCE: overlayfs: Propogate nosuid from lower and upper mounts
6b833b0 UBUNTU: SAUCE: overlayfs: Be more careful about copying up sxid files
9de69709 UBUNTU: SAUCE: overlayfs: Skip permission checking for trusted.overlayfs.* xattrs
21b8f14 UBUNTU: SAUCE: overlayfs: Use mounter's credentials instead of selectively raising caps
0c29f9e UBUNTU: SAUCE: overlayfs: Enable user namespace mounts for the "overlay" fstype
98a3740 UBUNTU: SAUCE: overlayfs: when copying up and reading directories ensure mounter had permissions V2
204bb1c UBUNTU: SAUCE: overlay: add backwards compatible overlayfs format support V4

Revision history for this message
Joseph Salisbury (jsalisbury) wrote :

Commits from upstream between -8 and -10:
8373f65 ovl: setattr: check permissions before copy-up
7193e80 ovl: root: copy attr
367e439 ovl: check dentry positiveness in ovl_cleanup_whiteouts()
fa93219 ovl: use a minimal buffer in ovl_copy_xattr
85a7ed3 ovl: allow zero size xattr

Revision history for this message
Seth Forshee (sforshee) wrote :

In the steps from comment #3, after the chown ls shows the file in both the upperdir and the mount as owned by $USER:$USER. However the inode from the overlayfs superblock which ovl_setattr() passes to inode_change_ok() seems to still be owned by root. This seems to be because ovl_getattr() gets the attributes from the inode in the upper/lower fs and not from the overlayfs inode.

Actually, I don't think the attributes were ever being copied to the overlayfs inode. "ovl: setattr: check permissions before copy-up" adds a call to inode_change_ok() on the overlayfs inode at the beginning of ovl_setattr(). Before that it only ever checked that the change was okay for the inode in upperdir, so the fact that the attributes weren't copied over didn't cause a failure.

That's a commit we got from upstream stable, and sure enough the bug exists in 4.5-rc7 as well.

Changed in linux (Ubuntu Xenial):
assignee: nobody → Seth Forshee (sforshee)
status: Triaged → In Progress
Revision history for this message
Seth Forshee (sforshee) wrote :

Reverting that patch fixes the regression. I'll send the patch to revert and report the bug upstream.

Revision history for this message
Seth Forshee (sforshee) wrote :

Actually there's already a fix for this in Linus' tree which tests fine for me.

Seth Forshee (sforshee)
Changed in linux (Ubuntu Wily):
assignee: nobody → Seth Forshee (sforshee)
importance: Undecided → Medium
status: New → In Progress
Revision history for this message
Martin Pitt (pitti) wrote :

Nice work Seth, thanks!

Seth Forshee (sforshee)
Changed in linux (Ubuntu Xenial):
status: In Progress → Fix Committed
Brad Figg (brad-figg)
Changed in linux (Ubuntu Wily):
status: In Progress → Fix Committed
Revision history for this message
Launchpad Janitor (janitor) wrote :

This bug was fixed in the package linux - 4.4.0-14.30

---------------
linux (4.4.0-14.30) xenial; urgency=low

  [ Tim Gardner ]

  * Release Tracking Bug
    - LP: #1557508

  * Current 4.4 kernel won't boot on powerpc (LP: #1557130)
    - powerpc: Fix dedotify for binutils >= 2.26

  * ZFS: send fails to transmit some holes [corruption] (LP: #1557151)
    - Illumos 6370 - ZFS send fails to transmit some holes

  * Request to cherry-pick uvcvideo patch for Xenial kernel support of RealSense
    camera (LP: #1557138)
    - UVC: Add support for ds4 depth camera

  * use after free of task_struct->numa_faults in task_numa_find_cpu (LP: #1527643)
    - sched/numa: Fix use-after-free bug in the task_numa_compare

  * overlay fs regression: chmod fails with "Operation not permitted" on chowned
    files (LP: #1555997)
    - ovl: copy new uid/gid into overlayfs runtime inode

  * Miscellaneous Ubuntu changes
    - SAUCE: Dump stack when X.509 certificates cannot be loaded

 -- Tim Gardner <email address hidden> Mon, 14 Mar 2016 07:16:19 -0600

Changed in linux (Ubuntu Xenial):
status: Fix Committed → Fix Released
Revision history for this message
Kamal Mostafa (kamalmostafa) 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-wily' to 'verification-done-wily'.

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-wily
Revision history for this message
Seth Forshee (sforshee) wrote :

Verified that the test case from comment #3 passes with 4.2.0-35.40.

tags: added: verification-done-wily
removed: verification-needed-wily
Revision history for this message
Launchpad Janitor (janitor) wrote :
Download full text (20.9 KiB)

This bug was fixed in the package linux - 4.2.0-35.40

---------------
linux (4.2.0-35.40) wily; urgency=low

  [ Brad Figg ]

  * Release Tracking Bug
    - LP: #1557706

  [ Upstream Kernel Changes ]

  * Revert "workqueue: make sure delayed work run in local cpu"
    - LP: #1556269
  * Revert "ALSA: hda - Fix noise on Gigabyte Z170X mobo"
    - LP: #1556269
  * KVM: VMX: Fix host initiated access to guest MSR_TSC_AUX
    - LP: #1552592
  * locking/qspinlock: Move __ARCH_SPIN_LOCK_UNLOCKED to qspinlock_types.h
    - LP: #1545330
  * [media] usbvision fix overflow of interfaces array
    - LP: #1556269
  * [media] usbvision: fix crash on detecting device with invalid
    configuration
    - LP: #1556269
  * ASN.1: Fix non-match detection failure on data overrun
    - LP: #1556269
  * iw_cxgb3: Fix incorrectly returning error on success
    - LP: #1556269
  * EVM: Use crypto_memneq() for digest comparisons
    - LP: #1556269
  * vmstat: explicitly schedule per-cpu work on the CPU we need it to run
    on
    - LP: #1556269
  * x86/entry/compat: Add missing CLAC to entry_INT80_32
    - LP: #1556269
  * iio-light: Use a signed return type for ltr501_match_samp_freq()
    - LP: #1556269
  * iio: add IIO_TRIGGER dependency to STK8BA50
    - LP: #1556269
  * iio: add HAS_IOMEM dependency to VF610_ADC
    - LP: #1556269
  * iio: dac: mcp4725: set iio name property in sysfs
    - LP: #1556269
  * iommu/vt-d: Fix 64-bit accesses to 32-bit DMAR_GSTS_REG
    - LP: #1556269
  * iio: light: acpi-als: Report data as processed
    - LP: #1556269
  * iio:adc:ti_am335x_adc Fix buffered mode by identifying as software
    buffer.
    - LP: #1556269
  * ASoC: rt5645: fix the shift bit of IN1 boost
    - LP: #1556269
  * ARCv2: STAR 9000950267: Handle return from intr to Delay Slot #2
    - LP: #1556269
  * cgroup: make sure a parent css isn't offlined before its children
    - LP: #1556269
  * ARM: OMAP2+: Fix wait_dll_lock_timed for rodata
    - LP: #1556269
  * ARM: OMAP2+: Fix l2dis_3630 for rodata
    - LP: #1556269
  * ARM: OMAP2+: Fix save_secure_ram_context for rodata
    - LP: #1556269
  * ARM: OMAP2+: Fix l2_inv_api_params for rodata
    - LP: #1556269
  * ARM: OMAP2+: Fix ppa_zero_params and ppa_por_params for rodata
    - LP: #1556269
  * rtlwifi: rtl8821ae: Fix 5G failure when EEPROM is incorrectly encoded
    - LP: #1556269
  * PCI/AER: Flush workqueue on device remove to avoid use-after-free
    - LP: #1556269
  * ARM: dts: Fix wl12xx missing clocks that cause hangs
    - LP: #1556269
  * libata: disable forced PORTS_IMPL for >= AHCI 1.3
    - LP: #1556269
  * mac80211: Requeue work after scan complete for all VIF types.
    - LP: #1556269
  * rfkill: fix rfkill_fop_read wait_event usage
    - LP: #1556269
  * ARM: dts: at91: sama5d4: fix instance id of DBGU
    - LP: #1556269
  * ARM: dts: at91: sama5d4ek: add phy address and IRQ for macb0
    - LP: #1556269
  * ARM: dts: at91: sama5d4 xplained: fix phy0 IRQ type
    - LP: #1556269
  * crypto: shash - Fix has_key setting
    - LP: #1556269
  * Input: vmmouse - fix absolute device registration
    - LP: #1556269
  * spi: atmel: fix gpio chip-select in case of non-DT platform
    - LP: #1556269
  ...

Changed in linux (Ubuntu Wily):
status: Fix Committed → Fix Released
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

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