Project

General

Profile

Actions

Bug #14537

closed

UFS should not allow directories to be unlinked

Added by Joshua M. Clulow 7 months ago. Updated 7 months ago.

Status:
Closed
Priority:
Normal
Category:
filesystems (not ZFS)
Start date:
Due date:
% Done:

100%

Estimated time:
Difficulty:
Medium
Tags:
Gerrit CR:
External Bug:

Description

I expect to uncover a variety of important and popular facts about UNIX as we investigate this, but it seems not unreasonable to me. Unlinking a directory upsets UFS on several levels, and requires unmounting and using fsck to fix. We know it's bad, so we only allow root to do it by default, and we have a privilege you can drop that will even stop root doing it accidentally. Really, though, can we not simply ban it entirely?


Related issues

Blocks illumos gate - Feature #14545: make PRIV_SYS_LINKDIR obsoleteNew

Actions
Actions #2

Updated by Joshua M. Clulow 7 months ago

I have elected to put together a small and surgical patch for this, leaving behind an override that can be set in /etc/system if someone realises they need it back. I will file a follow-up ticket for a more comprehensive deprecation of the PRIV_SYS_LINKDIR privilege, including documentation updates and removing the override variable.

Actions #3

Updated by Joshua M. Clulow 7 months ago

Testing Notes

I created a UFS file system, then created a directory in it, then used unlink to try to remove it:

# mkfile 25m /var/tmp/ufs_image
# lofiadm -a /var/tmp/ufs_image
/dev/lofi/2

# newfs -o space -m 0 -i 4096 -b 4096 /dev/lofi/2
newfs: construct a new file system /dev/rlofi/2: (y/n)? y
/dev/rlofi/2:   51170 sectors in 85 cylinders of 1 tracks, 602 sectors
        25.0MB in 6 cyl groups (16 c/g, 4.70MB/g, 1152 i/g)
super-block backups (for fsck -F ufs -o b=#) at:
 32, 9664, 19296, 28928, 38560, 48192,

# mount -F ufs -o nologging /dev/lofi/2 /mnt

# df -h /mnt
Filesystem             Size   Used  Available Capacity  Mounted on
/dev/lofi/2          24.05M     5K     24.05M     1%    /mnt

# mkdir /mnt/dir
# truss -t unlink,rmdir rmdir /mnt/dir
rmdir("/mnt/dir")                               = 0

# mkdir /mnt/dir
# truss -t unlink,rmdir unlink /mnt/dir
unlink("/mnt/dir")                              Err#1 EPERM
unlink: Not owner

# dtrace -n 'sec*linkdir:return /pid == $target/ { trace(arg1); }' -c 'unlink /mnt/dir'
dtrace: description 'sec*linkdir:return ' matched 1 probe
unlink: Not owner
dtrace: pid 670 has exited
CPU     ID                    FUNCTION:NAME
 20  22523      secpolicy_fs_linkdir:return                 1

# umount /mnt
# fsck -y /dev/lofi/2
** /dev/rlofi/2
** Last Mounted on /mnt
** Phase 1 - Check Blocks and Sizes
** Phase 2 - Check Pathnames
** Phase 3a - Check Connectivity
** Phase 3b - Verify Shadows/ACLs
** Phase 4 - Check Reference Counts
** Phase 5 - Check Cylinder Groups
3 files, 6 used, 24626 free (6 frags, 6155 blocks, 0.0% fragmentation)
Actions #4

Updated by Electric Monk 7 months ago

  • Gerrit CR set to 2054
Actions #5

Updated by Joshua M. Clulow 7 months ago

  • Assignee set to Joshua M. Clulow
Actions #6

Updated by Joshua M. Clulow 7 months ago

Actions #7

Updated by Joshua M. Clulow 7 months ago

Testing Notes (Supplemental)

Note that before the change, unlinking the directory worked like so:

# mkdir /mnt/dir

# truss -t unlink,rmdir unlink /mnt/dir
unlink("/mnt/dir")                              = 0

# umount /mnt

# fsck -y /dev/lofi/1
** /dev/rlofi/1
** Last Mounted on /mnt
** Phase 1 - Check Blocks and Sizes
** Phase 2 - Check Pathnames
** Phase 3a - Check Connectivity
UNREF DIR  I=4  OWNER=root MODE=40755
SIZE=512 MTIME=Feb 27 03:18 2022
RECONNECT?  yes

DIR I=4 CONNECTED. PARENT WAS I=2

** Phase 3b - Verify Shadows/ACLs
** Phase 4 - Check Reference Counts
LINK COUNT lost+found I=3  OWNER=root MODE=40700
SIZE=4096 MTIME=Feb 27 03:18 2022  COUNT 2 SHOULD BE 3
ADJUST?  yes

LINK COUNT DIR I=4  OWNER=root MODE=40755
SIZE=512 MTIME=Feb 27 03:18 2022  COUNT 1 SHOULD BE 2
ADJUST?  yes

** Phase 5 - Check Cylinder Groups
3 files, 6 used, 24626 free (6 frags, 6155 blocks, 0.0% fragmentation)

***** FILE SYSTEM WAS MODIFIED *****
ORPHANED DIRECTORIES REATTACHED; DIR LINK COUNTS MAY NOT BE CORRECT.

***** PLEASE RERUN FSCK *****

# fsck -y /dev/lofi/1
** /dev/rlofi/1
** Last Mounted on /mnt
** Phase 1 - Check Blocks and Sizes
** Phase 2 - Check Pathnames
** Phase 3a - Check Connectivity
** Phase 3b - Verify Shadows/ACLs
** Phase 4 - Check Reference Counts
LINK COUNT DIR I=2  OWNER=root MODE=40755
SIZE=512 MTIME=Feb 27 03:18 2022  COUNT 4 SHOULD BE 3
ADJUST?  yes

** Phase 5 - Check Cylinder Groups
3 files, 6 used, 24626 free (6 frags, 6155 blocks, 0.0% fragmentation)

***** FILE SYSTEM WAS MODIFIED *****

# fsck -y /dev/lofi/1
** /dev/rlofi/1
** Last Mounted on /mnt
** Phase 1 - Check Blocks and Sizes
** Phase 2 - Check Pathnames
** Phase 3a - Check Connectivity
** Phase 3b - Verify Shadows/ACLs
** Phase 4 - Check Reference Counts
** Phase 5 - Check Cylinder Groups
3 files, 6 used, 24626 free (6 frags, 6155 blocks, 0.0% fragmentation)

Making sure the tuneable works:

# truss -t unlink,rmdir unlink /mnt/dir
unlink("/mnt/dir")                              Err#1 EPERM
unlink: Not owner

# mdb -kw
Loading modules: [ unix genunix specfs dtrace mac cpu.generic uppc apix scsi_vhci zfs ip hook neti sockfs arp usba smbios fctl stmf stmf_sbd mm lofs idm cpc crypto fcip fcp random ufs logindmux nsmb ptm smbsrv nfs sppp sd ]
> ::nm ! grep linkdir
0xfffffffffbccea0c|0x0000000000000004|OBJT |GLOB |0x0  |6       |priv_allow_linkdir
0xfffffffffba37320|0x000000000000002e|FUNC |GLOB |0x0  |1       |secpolicy_fs_linkdir
> priv_allow_linkdir/X
priv_allow_linkdir:
priv_allow_linkdir:             0
> priv_allow_linkdir/W 1
priv_allow_linkdir:             0               =       0x1
> $q

# truss -t unlink,rmdir unlink /mnt/dir
unlink("/mnt/dir")                              = 0
Actions #8

Updated by Electric Monk 7 months ago

  • Status changed from New to Closed
  • % Done changed from 0 to 100

git commit ad8f9d956254e0caad9e4f8c85217f97cbdcade2

commit  ad8f9d956254e0caad9e4f8c85217f97cbdcade2
Author: Joshua M. Clulow <josh@sysmgr.org>
Date:   2022-02-28T07:30:30.000Z

    14537 UFS should not allow directories to be unlinked
    Reviewed by: Juraj Lutter <juraj@lutter.sk>
    Reviewed by: Rich Lowe <richlowe@richlowe.net>
    Reviewed by: Robert Mustacchi <rm@fingolfin.org>
    Reviewed by: Toomas Soome <tsoome@me.com>
    Approved by: Dan McDonald <danmcd@joyent.com>

Actions

Also available in: Atom PDF