Bug 260081

Summary: bhyve: capsicum preventing blockif hole-punching detection
Product: Base System Reporter: Chuck Tuffli <chuck>
Component: bhyveAssignee: freebsd-virtualization (Nobody) <virtualization>
Status: Closed FIXED    
Severity: Affects Many People CC: emaste, khng, markj
Priority: ---    
Version: CURRENT   
Hardware: Any   
OS: Any   
Attachments:
Description Flags
patch to allow detection of hole-punching none

Description Chuck Tuffli freebsd_committer freebsd_triage 2021-11-27 21:16:21 UTC 
Formatting a file-backed NVMe device in bhyve fails because the blockif_candelete function returns FALSE on kernel which supports hole-punching:

# uname -mrsvK
FreeBSD 14.0-CURRENT FreeBSD 14.0-CURRENT main-81b22a9892 GENERIC  amd64 1400041

This appears to happen because the cap_right_init(3) call does not allow the use of fpathconf(2). The attached patch which adds the CAP_FPATHCONF right fixes this.
Comment 1 Chuck Tuffli freebsd_committer freebsd_triage 2021-11-27 21:17:44 UTC 
Created attachment 229760 [details]
patch to allow detection of hole-punching
Comment 2 Ka Ho Ng freebsd_committer freebsd_triage 2021-11-28 23:37:56 UTC 
Do you have an opened differential to that? LGTM.

Ka Ho
Comment 3 Mark Johnston freebsd_committer freebsd_triage 2021-11-29 18:38:19 UTC 
Looks good to me also, I think the patch can simply be committed.
Comment 4 Chuck Tuffli freebsd_committer freebsd_triage 2021-11-30 20:06:14 UTC 
In the past, the bhyve process has strongly recommend code reviews. In that spirit https://reviews.freebsd.org/D33203
Comment 5 commit-hook freebsd_committer freebsd_triage 2021-11-30 21:51:04 UTC 
A commit in branch main references this bug:

URL: https://cgit.FreeBSD.org/src/commit/?id=d8c1d7b6526c7063304cd1e938d12dec137454e2

commit d8c1d7b6526c7063304cd1e938d12dec137454e2
Author:     Chuck Tuffli <chuck@FreeBSD.org>
AuthorDate: 2021-12-01 05:07:32 +0000
Commit:     Chuck Tuffli <chuck@FreeBSD.org>
CommitDate: 2021-12-01 05:49:34 +0000

    bhyve blockif: fix blockif_candelete with Capsicum

    NVMe conformance tests for the Format command failed if the
    backing-storage for the bhyve device was a file instead of a Zvol. The
    tests (and the specification) expect a Format to destroy all previously
    written data. The bhyve NVMe emulation implements this by trimming /
    deallocating all data from the backing-storage.

    The blockif_candelete() function indicated the file did not support
    deallocation (i.e. fpathconf(..., _PC_DEALLOC_PRESENT) returned FALSE)
    even though the kernel supported file hole punching. This occurs on
    builds with Capsicum enabled because blockif did not allow the
    fpathconf(2) right.

    Fix is to add CAP_FPATHCONF to the cap_rights_init(3) call.

    PR:             260081
    Reviewed by:    allanjude, markj, jhb
    MFC after:      1 week
    Differential Revision:  https://reviews.freebsd.org/D33203

 usr.sbin/bhyve/block_if.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
Comment 6 commit-hook freebsd_committer freebsd_triage 2022-02-21 21:58:46 UTC 
A commit in branch stable/13 references this bug:

URL: https://cgit.FreeBSD.org/src/commit/?id=1685d8368c2f78012f6d2d045f29193156a8580d

commit 1685d8368c2f78012f6d2d045f29193156a8580d
Author:     Chuck Tuffli <chuck@FreeBSD.org>
AuthorDate: 2021-12-01 05:07:32 +0000
Commit:     Chuck Tuffli <chuck@FreeBSD.org>
CommitDate: 2022-02-22 03:23:16 +0000

    bhyve blockif: fix blockif_candelete with Capsicum

    NVMe conformance tests for the Format command failed if the
    backing-storage for the bhyve device was a file instead of a Zvol. The
    tests (and the specification) expect a Format to destroy all previously
    written data. The bhyve NVMe emulation implements this by trimming /
    deallocating all data from the backing-storage.

    The blockif_candelete() function indicated the file did not support
    deallocation (i.e. fpathconf(..., _PC_DEALLOC_PRESENT) returned FALSE)
    even though the kernel supported file hole punching. This occurs on
    builds with Capsicum enabled because blockif did not allow the
    fpathconf(2) right.

    Fix is to add CAP_FPATHCONF to the cap_rights_init(3) call.

    PR:             260081

    (cherry picked from commit d8c1d7b6526c7063304cd1e938d12dec137454e2)

 usr.sbin/bhyve/block_if.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
Comment 7 Chuck Tuffli freebsd_committer freebsd_triage 2022-06-09 00:42:15 UTC 
Can this be closed now?
Comment 8 Mark Johnston freebsd_committer freebsd_triage 2022-06-09 13:39:15 UTC 
(In reply to Chuck Tuffli from comment #7)
Looks like it, yes.