[RFC PATCH v2 0/4] btrfs-progs: build distinct binaries for specific btrfs subcommands

[RFC PATCH v2 0/4] btrfs-progs: build distinct binaries for specific btrfs subcommands

[Axel Burri]

This patch allows to build distinct binaries for specific btrfs
subcommands, e.g. "btrfs-subvolume-show" which would be identical to
"btrfs subvolume show".

Changes from v1 [1]:

 - No more need of generated c-file for each separated commands (all
   functionality has moved to Makefile).

 - More generic approach: match entry point declarations as well as
   additional tage in all "cmds-*.c" files.

 - Change naming: use "separated" instead of "splitcmd".


Motivation:

While btrfs-progs offer the all-inclusive "btrfs" command, it gets
pretty cumbersome to restrict privileges to the subcommands [2].
Common approaches are to either setuid root for "/sbin/btrfs" (which
is not recommended at all), or to write sudo rules for each
subcommand.

Separating the subcommands into distinct binaries makes it easy to set
elevated privileges using capabilities(7) or setuid. A typical use
case where this is needed is when it comes to automated scripts,
e.g. btrbk [3] [4] creating snapshots and send/receive them via ssh.


Description:

Patch 1 adds a minimal, non-invasive framework for building separated
binaries. Note that some subcommands fail to build ("make -k separated").

Patches 2,3 fix build dependencies: make all subcommands build
correctly, with smaller binary size. Probably to be squashed into
patch 1 for final commit.

Patch 4 adds configuration options -enable-setcap-install,
--enable-setuid-install, --enable-btrfs-separated.


Notes:

 - This patchset is available on github [5].

 - A gentoo ebuild "sys-fs/btrfs-progs-separated" is available on
   github [6], as well as in the digint-overlay [7]:

    USE=filecaps emerge -av sys-fs/btrfs-progs-separated


References:

  [1] https://www.spinics.net/lists/linux-btrfs/msg81451.html
  [2] https://www.spinics.net/lists/linux-btrfs/msg75736.html
  [3] https://github.com/digint/btrbk
  [4] https://github.com/digint/btrfs-progs-btrbk
  [5] https://github.com/digint/btrfs-progs/tree/cmds-separated-fscaps-v2
  [6] https://github.com/digint/gentoo/tree/btrfs-progs-separated/sys-fs/btrfs-progs-separated
  [7] https://dev.tty0.ch/portage/digint-overlay.git


Axel Burri (4):
  btrfs-progs: Makefile: create separated binaries for "btrfs"
    subcommands; add fscaps declarations
  btrfs-progs: remove unneeded dependencies on separated build
    (-DBTRFS_SEPARATED_BUILD)
  btrfs-progs: Makefile: add extra objects definitions for separated
    binaries
  btrfs-progs: build: add --enable-setcap-install,
    --enable-setuid-install, --enable-btrfs-separated

 .gitignore        |  1 +
 Makefile          | 99 ++++++++++++++++++++++++++++++++++++++++++++++++++++++-
 Makefile.inc.in   |  6 ++++
 cmds-balance.c    |  2 ++
 cmds-device.c     |  2 ++
 cmds-fi-usage.c   |  1 +
 cmds-filesystem.c |  2 ++
 cmds-inspect.c    |  2 ++
 cmds-property.c   |  2 ++
 cmds-qgroup.c     |  3 ++
 cmds-quota.c      |  2 ++
 cmds-receive.c    |  1 +
 cmds-replace.c    |  2 ++
 cmds-rescue.c     |  2 ++
 cmds-scrub.c      |  2 ++
 cmds-send.c       |  1 +
 cmds-subvolume.c  |  6 ++++
 commands.h        | 37 +++++++++++++++++++++
 configure.ac      | 40 ++++++++++++++++++++++
 19 files changed, 212 insertions(+), 1 deletion(-)

-- 
2.16.4

[RFC PATCH v2 1/4] btrfs-progs: Makefile: create separated binaries for "btrfs" subcommands; add fscaps declarations

[Axel Burri]

Create separate binaries "btrfs-xxx-yyy.separated" for each subcommand
"btrfs xxx yyy". Also declares fscaps for (supported) subcommands.
This is useful for admins to provide specific subcommand binaries with
elevated privileges (capabilities(7), suid).

Example:

    # make separated-fscaps
    # make list-fscaps
    # ./btrfs-subvolume-list.separated
    # ./btrfs-send.separated
    <...>
    # make -k separated

A list of subcommands is assembled in the makefile by matching main
entry points and generic "@SEPARATED" tags in "cmds-*.c", e.g.:

    // @SEPARATED btrfs-subvolume-delete fscaps: cap_sys_admin,cap_dac_override
    static int cmd_subvol_delete(int argc, char **argv)

A patch in "commands.h" adds a "int main()" symbol and tweaks for
building separated binaries if BTRFS_SEPARATED_BUILD and
BTRFS_SEPARATED_ENTRY are defined. These defines are set generically
for each subcommand in the "btrfs-%.separated.o" makefile target.

Makefile targets:

 - "btrfs-%.separated": builds a separated binary for a specific btrfs
   subcommand (see "make list-separated").

 - `make -k separated`: builds all separated (btrfs-xxx-yyy.separated)
   binaries. Note: some targets fail with "ld: undefined reference"
   errors (will be fixed in later commit).

 - `make separated-fscaps`: build separated binaries for subcommands
   declaring "@SEPARATED btrfs-xxx fscaps" (comments) within the
   c-file (for sysadmins / package maintainer).

 - `make list-separated`: print a list of supported subcommands.

 - `make list-fscaps`: print fscaps required for setcap(8). Note that
   this list only covers some basic subcommands (the ones used by
   btrbk-0.26.1 "backend btrfs-progs-btrbk") and needs to be improved.

Signed-off-by: Axel Burri <axel@tty0.ch>
---
 .gitignore       |  1 +
 Makefile         | 53 +++++++++++++++++++++++++++++++++++++++++++++++++++++
 cmds-fi-usage.c  |  1 +
 cmds-qgroup.c    |  1 +
 cmds-receive.c   |  1 +
 cmds-send.c      |  1 +
 cmds-subvolume.c |  4 ++++
 commands.h       | 45 +++++++++++++++++++++++++++++++++++++++++++++
 8 files changed, 107 insertions(+)

diff --git a/.gitignore b/.gitignore
index 144ebb3b..e1eb1341 100644
--- a/.gitignore
+++ b/.gitignore
@@ -10,6 +10,7 @@ Documentation/*.gz
 Documentation/*.html
 btrfs
 btrfs.static
+btrfs-*.separated
 btrfs-map-logical
 btrfs-fragments
 btrfsck
diff --git a/Makefile b/Makefile
index fcfc815a..35666ec9 100644
--- a/Makefile
+++ b/Makefile
@@ -2,6 +2,7 @@
 # Basic build targets:
 #   all		all main tools and the shared library
 #   static      build static bnaries, requires static version of the libraries
+#   separated   build separated binary for each "btrfs" subcommand
 #   test        run the full testsuite
 #   install     install to default location (/usr/local)
 #   clean       clean built binaries (not the documentation)
@@ -231,6 +232,27 @@ progs_install =
 progs_build =
 endif
 
+# Parse "int cmd_xxx_yyy(int argc, char **argv)" lines in cfiles, and
+# create whitespace separated map of form: "btrfs-xxx-yyy@key:value".
+sc_cfiles := $(wildcard cmds-*.c)
+sc_map := $(foreach file,$(sc_cfiles),$(shell sed -rn 's/^(static )?int cmd_([a-z_]+)\(int argc, char.*argv.*\)$$/btrfs_\2@cfile:$(file) btrfs_\2@entry:cmd_\2 btrfs_\2@static_entry:\1/p' $(file)))
+sc_map := $(foreach val,$(sc_map),$(subst _,-,$(firstword $(subst @, ,$(val))))@$(word 2,$(subst @, ,$(val))))
+sc_map := $(subst btrfs-subvol-,btrfs-subvolume-,$(sc_map))
+
+# Parse generic "@SEPARATED btrfs-xxx-yyy key: value" in cfiles, and add to map.
+sc_map += $(shell sed -rn 's/^.*@SEPARATED\s+(btrfs-[a-z-]+)\s+([a-z_]+):\s*(.*)$$/\1@\2:\3/p' $(sc_cfiles))
+sc_get = $(word 2,$(subst :, ,$(filter $(1)@$(2):%,$(sc_map))))
+
+# Exclude first-level commands relying on "int handle_command_group()" or buggy
+sc_exclude = btrfs-balance btrfs-balance-full btrfs-device btrfs-filesystem btrfs-inspect btrfs-property btrfs-qgroup btrfs-quota btrfs-replace btrfs-rescue btrfs-scrub btrfs-subvolume
+sc_cmds_all := $(sort $(foreach val,$(sc_map),$(firstword $(subst @, ,$(val)))))
+sc_cmds := $(filter-out $(sc_exclude),$(sc_cmds_all))
+sc_cmds_fscaps := $(sort $(subst @fscaps,,$(filter %@fscaps,$(subst :, ,$(sc_map)))))
+
+# Using suffix allows strict distinction in targets below (btrfs-%.separated[.o])
+progs_separated = $(addsuffix .separated,$(sc_cmds))
+progs_separated_fscaps = $(addsuffix .separated,$(sc_cmds_fscaps))
+
 # external libs required by various binaries; for btrfs-foo,
 # specify btrfs_foo_libs = <list of libs>; see $($(subst...)) rules below
 btrfs_convert_cflags = -DBTRFSCONVERT_EXT2=$(BTRFSCONVERT_EXT2)
@@ -312,6 +334,17 @@ endif
 	$(Q)$(CC) $(CFLAGS) -c $< -o $@ $($(subst -,_,$(@:%.o=%)-cflags)) \
 		$($(subst -,_,btrfs-$(@:%/$(notdir $@)=%)-cflags))
 
+# Compile target objects providing main() symbol (see commands.h:
+# BTRFS_SEPARATED_BUILD), using "cfile" from sc_map (cmd-xxx.c) as gcc infile.
+btrfs-%.separated.o: $(call sc_get,$(@:%.separated.o=%),cfile)
+	@echo "    [CC]     $@"
+	$(Q)$(CC) $(CFLAGS) \
+		-DBTRFS_SEPARATED_BUILD \
+		-DBTRFS_SEPARATED_ENTRY=$(call sc_get,$(@:%.separated.o=%),entry) \
+		-DBTRFS_SEPARATED_USAGE=$(call sc_get,$(@:%.separated.o=%),entry)_usage \
+		$(if $(call sc_get,$(@:%.separated.o=%),static_entry),-DBTRFS_SEPARATED_STATIC_ENTRY) \
+		-c $(call sc_get,$(@:%.separated.o=%),cfile) -o $@
+
 %.static.o: %.c
 	@echo "    [CC]     $@"
 	$(Q)$(CC) $(STATIC_CFLAGS) -c $< -o $@ $($(subst -,_,$(@:%.static.o=%)-cflags)) \
@@ -384,6 +417,20 @@ endif
 #
 static: $(progs_static)
 
+separated: $(progs_separated)
+
+separated-fscaps: $(progs_separated_fscaps)
+
+.PHONY: separated separated-fscaps
+
+list-separated:
+	@echo "$(sc_cmds)" | tr ' ' '\n'
+
+list-fscaps:
+	@echo "$(foreach sc,$(sc_cmds_fscaps),$(sc)=$(call sc_get,$(sc),fscaps))" | tr ' ' '\n'
+
+.PHONY: list-separated list-fscaps
+
 version.h: version.h.in configure.ac
 	@echo "    [SH]     $@"
 	$(Q)bash ./config.status --silent $@
@@ -454,6 +501,11 @@ btrfs-%.static: btrfs-%.static.o $(static_objects) $(patsubst %.o,%.static.o,$(s
 		$(static_libbtrfs_objects) $(STATIC_LDFLAGS) \
 		$($(subst -,_,$(subst .static,,$@)-libs)) $(STATIC_LIBS)
 
+btrfs-%.separated: btrfs-%.separated.o $(objects) $(cmds_objects) $(libs_static)
+	@echo "    [LD]     $@"
+	$(Q)$(CC) -o $@ $@.o $(objects) $(libs_static) \
+		$(LDFLAGS) $(LIBS)
+
 btrfs-%: btrfs-%.o $(objects) $(standalone_deps) $(libs_static)
 	@echo "    [LD]     $@"
 	$(Q)$(CC) -o $@ $(objects) $@.o \
@@ -618,6 +670,7 @@ clean: $(CLEANDIRS)
 	      $(check_defs) \
 	      $(libs) $(lib_links) \
 	      $(progs_static) \
+	      $(progs_separated) \
 	      libbtrfsutil/*.o libbtrfsutil/*.o.d
 ifeq ($(PYTHON_BINDINGS),1)
 	$(Q)cd libbtrfsutil/python; \
diff --git a/cmds-fi-usage.c b/cmds-fi-usage.c
index dca2e8d0..f44dfe54 100644
--- a/cmds-fi-usage.c
+++ b/cmds-fi-usage.c
@@ -964,6 +964,7 @@ const char * const cmd_filesystem_usage_usage[] = {
 	NULL
 };
 
+// @SEPARATED btrfs-filesystem-usage fscaps: cap_sys_admin
 int cmd_filesystem_usage(int argc, char **argv)
 {
 	int ret = 0;
diff --git a/cmds-qgroup.c b/cmds-qgroup.c
index b928edc7..554f2bf2 100644
--- a/cmds-qgroup.c
+++ b/cmds-qgroup.c
@@ -259,6 +259,7 @@ static const char * const cmd_qgroup_destroy_usage[] = {
 	NULL
 };
 
+// @SEPARATED btrfs-qgroup-destroy fscaps: cap_sys_admin,cap_dac_override
 static int cmd_qgroup_destroy(int argc, char **argv)
 {
 	int ret;
diff --git a/cmds-receive.c b/cmds-receive.c
index 34d51ef3..04aaad6c 100644
--- a/cmds-receive.c
+++ b/cmds-receive.c
@@ -1248,6 +1248,7 @@ out:
 	return ret;
 }
 
+// @SEPARATED btrfs-receive fscaps: cap_sys_admin,cap_fowner,cap_chown,cap_mknod,cap_setfcap,cap_dac_override,cap_dac_read_search
 int cmd_receive(int argc, char **argv)
 {
 	char *tomnt = NULL;
diff --git a/cmds-send.c b/cmds-send.c
index 16b9f8d2..259faeff 100644
--- a/cmds-send.c
+++ b/cmds-send.c
@@ -489,6 +489,7 @@ static void free_send_info(struct btrfs_send *sctx)
 	subvol_uuid_search_finit(&sctx->sus);
 }
 
+// @SEPARATED btrfs-send fscaps: cap_sys_admin,cap_fowner,cap_dac_read_search
 int cmd_send(int argc, char **argv)
 {
 	char *subvol = NULL;
diff --git a/cmds-subvolume.c b/cmds-subvolume.c
index e7a884af..a4202ec9 100644
--- a/cmds-subvolume.c
+++ b/cmds-subvolume.c
@@ -231,6 +231,7 @@ static const char * const cmd_subvol_delete_usage[] = {
 	NULL
 };
 
+// @SEPARATED btrfs-subvolume-delete fscaps: cap_sys_admin,cap_dac_override
 static int cmd_subvol_delete(int argc, char **argv)
 {
 	int res, ret = 0;
@@ -451,6 +452,7 @@ static const char * const cmd_subvol_list_usage[] = {
 	NULL,
 };
 
+// @SEPARATED btrfs-subvolume-list fscaps: cap_sys_admin,cap_fowner,cap_dac_read_search
 static int cmd_subvol_list(int argc, char **argv)
 {
 	struct btrfs_list_filter_set *filter_set;
@@ -623,6 +625,7 @@ static const char * const cmd_subvol_snapshot_usage[] = {
 	NULL
 };
 
+// @SEPARATED btrfs-subvolume-snapshot fscaps: cap_sys_admin,cap_fowner,cap_dac_override,cap_dac_read_search
 static int cmd_subvol_snapshot(int argc, char **argv)
 {
 	char	*subvol, *dst;
@@ -920,6 +923,7 @@ static const char * const cmd_subvol_show_usage[] = {
 	NULL
 };
 
+// @SEPARATED btrfs-subvolume-show fscaps: cap_sys_admin,cap_fowner,cap_dac_read_search
 static int cmd_subvol_show(int argc, char **argv)
 {
 	char tstr[256];
diff --git a/commands.h b/commands.h
index 76991f2b..c9901de9 100644
--- a/commands.h
+++ b/commands.h
@@ -121,4 +121,49 @@ int cmd_dump_super(int argc, char **argv);
 int cmd_debug_tree(int argc, char **argv);
 int cmd_rescue(int argc, char **argv);
 
+
+#ifdef BTRFS_SEPARATED_BUILD
+
+#ifndef BTRFS_SEPARATED_ENTRY
+#error please define BTRFS_SEPARATED_ENTRY (see Makefile: "btrfs-%.separated.o" target)
+#endif
+
+#ifdef BTRFS_SEPARATED_USAGE
+#include "help.h"
+#endif
+
+/* Note: handle_command_group is defined in btrfs.c and cannot be
+ * linked with separated subcommands because btrfs.o also contains a
+ * "main" symbol. As a workaround, we simply return 1 (error) for
+ * calls to handle_command_group() here (which is fine as this
+ * functionality is not required for BTRFS_SEPARATED_BUILD commands).
+ */
+#define handle_command_group(cmd_group,argc,argv) 1
+
+/* forward declaration of main entry point (non-static are already declared above) */
+#ifdef BTRFS_SEPARATED_STATIC_ENTRY
+static int BTRFS_SEPARATED_ENTRY(int argc, char **argv);
+static const char * const BTRFS_SEPARATED_USAGE [];
+#endif
+
+int main(int argc, char **argv)
+{
+	int i;
+	for (i = 1; i < argc; i++) {
+#ifdef BTRFS_SEPARATED_USAGE
+		if (strcmp(argv[i], "--help") == 0) {
+			usage(BTRFS_SEPARATED_USAGE);
+			return 0;
+		}
+#endif
+		if (strcmp(argv[i], "--version") == 0) {
+			printf("%s\n", PACKAGE_STRING);
+			return 0;
+		}
+	}
+	return BTRFS_SEPARATED_ENTRY (argc, argv);
+}
+
+#endif  /* BTRFS_SEPARATED_BUILD */
+
 #endif
-- 
2.16.4

[RFC PATCH v2 2/4] btrfs-progs: remove unneeded dependencies on separated build (-DBTRFS_SEPARATED_BUILD)

[Axel Burri]

Remove references to unneeded symbols when building separated
subcommands: "btrfs-*.separated.o".

Note that this patch still leaves unreferenced static symbols, which
in turn makes the compiler warn about "unused function/variable".
Stripping all symbols would imply adaptions in the source code
(especially by moving "int handle_command_group" functionality into a
separate compile unit).

This is not a problem, ignore these warnings when building object:

    $(CC) -Wno-unused-function -Wno-unused-const-variable

Signed-off-by: Axel Burri <axel@tty0.ch>
---
 Makefile          | 2 +-
 cmds-balance.c    | 2 ++
 cmds-device.c     | 2 ++
 cmds-filesystem.c | 2 ++
 cmds-inspect.c    | 2 ++
 cmds-property.c   | 2 ++
 cmds-qgroup.c     | 2 ++
 cmds-quota.c      | 2 ++
 cmds-replace.c    | 2 ++
 cmds-rescue.c     | 2 ++
 cmds-scrub.c      | 2 ++
 cmds-subvolume.c  | 2 ++
 commands.h        | 8 --------
 13 files changed, 23 insertions(+), 9 deletions(-)

diff --git a/Makefile b/Makefile
index 35666ec9..95db571b 100644
--- a/Makefile
+++ b/Makefile
@@ -338,7 +338,7 @@ endif
 # BTRFS_SEPARATED_BUILD), using "cfile" from sc_map (cmd-xxx.c) as gcc infile.
 btrfs-%.separated.o: $(call sc_get,$(@:%.separated.o=%),cfile)
 	@echo "    [CC]     $@"
-	$(Q)$(CC) $(CFLAGS) \
+	$(Q)$(CC) $(CFLAGS) -Wno-unused-function -Wno-unused-const-variable \
 		-DBTRFS_SEPARATED_BUILD \
 		-DBTRFS_SEPARATED_ENTRY=$(call sc_get,$(@:%.separated.o=%),entry) \
 		-DBTRFS_SEPARATED_USAGE=$(call sc_get,$(@:%.separated.o=%),entry)_usage \
diff --git a/cmds-balance.c b/cmds-balance.c
index 6cc26c35..72292bc8 100644
--- a/cmds-balance.c
+++ b/cmds-balance.c
@@ -914,6 +914,7 @@ static int cmd_balance_full(int argc, char **argv)
 static const char balance_cmd_group_info[] =
 "balance data across devices, or change block groups using filters";
 
+#ifndef BTRFS_SEPARATED_BUILD
 const struct cmd_group balance_cmd_group = {
 	balance_cmd_group_usage, balance_cmd_group_info, {
 		{ "start", cmd_balance_start, cmd_balance_start_usage, NULL, 0 },
@@ -940,3 +941,4 @@ int cmd_balance(int argc, char **argv)
 
 	return handle_command_group(&balance_cmd_group, argc, argv);
 }
+#endif
diff --git a/cmds-device.c b/cmds-device.c
index 2a05f70a..719afc01 100644
--- a/cmds-device.c
+++ b/cmds-device.c
@@ -597,6 +597,7 @@ static int cmd_device_usage(int argc, char **argv)
 static const char device_cmd_group_info[] =
 "manage and query devices in the filesystem";
 
+#ifndef BTRFS_SEPARATED_BUILD
 const struct cmd_group device_cmd_group = {
 	device_cmd_group_usage, device_cmd_group_info, {
 		{ "add", cmd_device_add, cmd_device_add_usage, NULL, 0 },
@@ -616,3 +617,4 @@ int cmd_device(int argc, char **argv)
 {
 	return handle_command_group(&device_cmd_group, argc, argv);
 }
+#endif
diff --git a/cmds-filesystem.c b/cmds-filesystem.c
index 06c8311b..be5d50f2 100644
--- a/cmds-filesystem.c
+++ b/cmds-filesystem.c
@@ -1182,6 +1182,7 @@ static int cmd_filesystem_label(int argc, char **argv)
 static const char filesystem_cmd_group_info[] =
 "overall filesystem tasks and information";
 
+#ifndef BTRFS_SEPARATED_BUILD
 const struct cmd_group filesystem_cmd_group = {
 	filesystem_cmd_group_usage, filesystem_cmd_group_info, {
 		{ "df", cmd_filesystem_df, cmd_filesystem_df_usage, NULL, 0 },
@@ -1209,3 +1210,4 @@ int cmd_filesystem(int argc, char **argv)
 {
 	return handle_command_group(&filesystem_cmd_group, argc, argv);
 }
+#endif
diff --git a/cmds-inspect.c b/cmds-inspect.c
index ac77a5ee..1ad48548 100644
--- a/cmds-inspect.c
+++ b/cmds-inspect.c
@@ -634,6 +634,7 @@ out:
 static const char inspect_cmd_group_info[] =
 "query various internal information";
 
+#ifndef BTRFS_SEPARATED_BUILD
 const struct cmd_group inspect_cmd_group = {
 	inspect_cmd_group_usage, inspect_cmd_group_info, {
 		{ "inode-resolve", cmd_inspect_inode_resolve,
@@ -660,3 +661,4 @@ int cmd_inspect(int argc, char **argv)
 {
 	return handle_command_group(&inspect_cmd_group, argc, argv);
 }
+#endif
diff --git a/cmds-property.c b/cmds-property.c
index 03bafa05..c98e8ebb 100644
--- a/cmds-property.c
+++ b/cmds-property.c
@@ -408,6 +408,7 @@ static int cmd_property_list(int argc, char **argv)
 static const char property_cmd_group_info[] =
 "modify properties of filesystem objects";
 
+#ifndef BTRFS_SEPARATED_BUILD
 const struct cmd_group property_cmd_group = {
 	property_cmd_group_usage, property_cmd_group_info, {
 		{ "get", cmd_property_get,
@@ -424,3 +425,4 @@ int cmd_property(int argc, char **argv)
 {
 	return handle_command_group(&property_cmd_group, argc, argv);
 }
+#endif
diff --git a/cmds-qgroup.c b/cmds-qgroup.c
index 554f2bf2..5f5a3563 100644
--- a/cmds-qgroup.c
+++ b/cmds-qgroup.c
@@ -503,6 +503,7 @@ static int cmd_qgroup_limit(int argc, char **argv)
 static const char qgroup_cmd_group_info[] =
 "manage quota groups";
 
+#ifndef BTRFS_SEPARATED_BUILD
 const struct cmd_group qgroup_cmd_group = {
 	qgroup_cmd_group_usage, qgroup_cmd_group_info, {
 		{ "assign", cmd_qgroup_assign, cmd_qgroup_assign_usage,
@@ -525,3 +526,4 @@ int cmd_qgroup(int argc, char **argv)
 {
 	return handle_command_group(&qgroup_cmd_group, argc, argv);
 }
+#endif
diff --git a/cmds-quota.c b/cmds-quota.c
index c9ea9c0f..8585d427 100644
--- a/cmds-quota.c
+++ b/cmds-quota.c
@@ -194,6 +194,7 @@ static int cmd_quota_rescan(int argc, char **argv)
 static const char quota_cmd_group_info[] =
 "manage filesystem quota settings";
 
+#ifndef BTRFS_SEPARATED_BUILD
 const struct cmd_group quota_cmd_group = {
 	quota_cmd_group_usage, quota_cmd_group_info, {
 		{ "enable", cmd_quota_enable, cmd_quota_enable_usage, NULL, 0 },
@@ -208,3 +209,4 @@ int cmd_quota(int argc, char **argv)
 {
 	return handle_command_group(&quota_cmd_group, argc, argv);
 }
+#endif
diff --git a/cmds-replace.c b/cmds-replace.c
index 1fa80284..44456c4a 100644
--- a/cmds-replace.c
+++ b/cmds-replace.c
@@ -545,6 +545,7 @@ static int cmd_replace_cancel(int argc, char **argv)
 static const char replace_cmd_group_info[] =
 "replace a device in the filesystem";
 
+#ifndef BTRFS_SEPARATED_BUILD
 const struct cmd_group replace_cmd_group = {
 	replace_cmd_group_usage, replace_cmd_group_info, {
 		{ "start", cmd_replace_start, cmd_replace_start_usage, NULL,
@@ -561,3 +562,4 @@ int cmd_replace(int argc, char **argv)
 {
 	return handle_command_group(&replace_cmd_group, argc, argv);
 }
+#endif
diff --git a/cmds-rescue.c b/cmds-rescue.c
index 38c4ab9b..bdc6dd4b 100644
--- a/cmds-rescue.c
+++ b/cmds-rescue.c
@@ -253,6 +253,7 @@ out:
 static const char rescue_cmd_group_info[] =
 "toolbox for specific rescue operations";
 
+#ifndef BTRFS_SEPARATED_BUILD
 const struct cmd_group rescue_cmd_group = {
 	rescue_cmd_group_usage, rescue_cmd_group_info, {
 		{ "chunk-recover", cmd_rescue_chunk_recover,
@@ -270,3 +271,4 @@ int cmd_rescue(int argc, char **argv)
 {
 	return handle_command_group(&rescue_cmd_group, argc, argv);
 }
+#endif
diff --git a/cmds-scrub.c b/cmds-scrub.c
index 6b909f20..e418f907 100644
--- a/cmds-scrub.c
+++ b/cmds-scrub.c
@@ -1788,6 +1788,7 @@ out:
 static const char scrub_cmd_group_info[] =
 "verify checksums of data and metadata";
 
+#ifndef BTRFS_SEPARATED_BUILD
 const struct cmd_group scrub_cmd_group = {
 	scrub_cmd_group_usage, scrub_cmd_group_info, {
 		{ "start", cmd_scrub_start, cmd_scrub_start_usage, NULL, 0 },
@@ -1802,3 +1803,4 @@ int cmd_scrub(int argc, char **argv)
 {
 	return handle_command_group(&scrub_cmd_group, argc, argv);
 }
+#endif
diff --git a/cmds-subvolume.c b/cmds-subvolume.c
index a4202ec9..15dbdf3a 100644
--- a/cmds-subvolume.c
+++ b/cmds-subvolume.c
@@ -1226,6 +1226,7 @@ out:
 static const char subvolume_cmd_group_info[] =
 "manage subvolumes: create, delete, list, etc";
 
+#ifndef BTRFS_SEPARATED_BUILD
 const struct cmd_group subvolume_cmd_group = {
 	subvolume_cmd_group_usage, subvolume_cmd_group_info, {
 		{ "create", cmd_subvol_create, cmd_subvol_create_usage, NULL, 0 },
@@ -1249,3 +1250,4 @@ int cmd_subvolume(int argc, char **argv)
 {
 	return handle_command_group(&subvolume_cmd_group, argc, argv);
 }
+#endif
diff --git a/commands.h b/commands.h
index c9901de9..35081b94 100644
--- a/commands.h
+++ b/commands.h
@@ -132,14 +132,6 @@ int cmd_rescue(int argc, char **argv);
 #include "help.h"
 #endif
 
-/* Note: handle_command_group is defined in btrfs.c and cannot be
- * linked with separated subcommands because btrfs.o also contains a
- * "main" symbol. As a workaround, we simply return 1 (error) for
- * calls to handle_command_group() here (which is fine as this
- * functionality is not required for BTRFS_SEPARATED_BUILD commands).
- */
-#define handle_command_group(cmd_group,argc,argv) 1
-
 /* forward declaration of main entry point (non-static are already declared above) */
 #ifdef BTRFS_SEPARATED_STATIC_ENTRY
 static int BTRFS_SEPARATED_ENTRY(int argc, char **argv);
-- 
2.16.4

[RFC PATCH v2 3/4] btrfs-progs: Makefile: add extra objects definitions for separated binaries

[Axel Burri]

Some separated binaries have references to specific command objects
($cmds_objects). Add these dependencies in the Makefile, and use them
in the linker target (as in target "btrfs-%:").

Fixes linkage errors for these subcommands. The "make separated"
target now builds without errors.

Signed-off-by: Axel Burri <axel@tty0.ch>
---
 Makefile | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/Makefile b/Makefile
index 95db571b..362550c9 100644
--- a/Makefile
+++ b/Makefile
@@ -249,6 +249,15 @@ sc_cmds_all := $(sort $(foreach val,$(sc_map),$(firstword $(subst @, ,$(val)))))
 sc_cmds := $(filter-out $(sc_exclude),$(sc_cmds_all))
 sc_cmds_fscaps := $(sort $(subst @fscaps,,$(filter %@fscaps,$(subst :, ,$(sc_map)))))
 
+# Additional cmds_objects required for separated binaries
+btrfs_device_add_objects = mkfs/common.o
+btrfs_device_usage_objects = cmds-fi-usage.o
+btrfs_filesystem_show_objects = cmds-fi-usage.o
+btrfs_replace_start_objects = mkfs/common.o
+btrfs_rescue_super_recover_objects = super-recover.o
+btrfs_rescue_chunk_recover_objects = check/mode-lowmem.o check/mode-common.o check/main.o chunk-recover.o
+btrfs_restore_objects = $(LIBS_COMP)
+
 # Using suffix allows strict distinction in targets below (btrfs-%.separated[.o])
 progs_separated = $(addsuffix .separated,$(sc_cmds))
 progs_separated_fscaps = $(addsuffix .separated,$(sc_cmds_fscaps))
@@ -504,6 +513,7 @@ btrfs-%.static: btrfs-%.static.o $(static_objects) $(patsubst %.o,%.static.o,$(s
 btrfs-%.separated: btrfs-%.separated.o $(objects) $(cmds_objects) $(libs_static)
 	@echo "    [LD]     $@"
 	$(Q)$(CC) -o $@ $@.o $(objects) $(libs_static) \
+		$($(subst -,_,$(@:%.separated=%)-objects)) \
 		$(LDFLAGS) $(LIBS)
 
 btrfs-%: btrfs-%.o $(objects) $(standalone_deps) $(libs_static)
-- 
2.16.4

[RFC PATCH v2 4/4] btrfs-progs: build: add --enable-setcap-install, --enable-setuid-install, --enable-btrfs-separated

[Axel Burri]

Adds Makefile target "install-separated": install all
"btrfs-*.separated" binaries and rename to "btrfs-*". If configured
with --enable-setcap-install, also sets linux capabilities(7) using
setcap(8). If configured with "--enable-setuid-install", sets setuid
bit while installing.

Use --enable-btrfs-separated if you want to build (but not install)
all "btrfs-*.separated" binaries.

Signed-off-by: Axel Burri <axel@tty0.ch>
---
 Makefile        | 36 +++++++++++++++++++++++++++++++++++-
 Makefile.inc.in |  6 ++++++
 configure.ac    | 40 ++++++++++++++++++++++++++++++++++++++++
 3 files changed, 81 insertions(+), 1 deletion(-)

diff --git a/Makefile b/Makefile
index 362550c9..e0edcb75 100644
--- a/Makefile
+++ b/Makefile
@@ -232,6 +232,25 @@ progs_install =
 progs_build =
 endif
 
+ifeq ($(BUILD_BTRFS_SEPARATED),1)
+# Note: intentionally not addded to progs_install:
+# use -enable-setcap-install, --enable-setuid-install instead.
+progs_build += $(progs_separated)
+endif
+
+INSTALL_SEPARATED_OPTIONS = -m755
+ifeq ($(ENABLE_INSTALL_SETCAP),1)
+INSTALL_SEPARATED_OPTIONS = -m710
+progs_install_separated += $(progs_separated_fscaps)
+endif
+ifeq ($(ENABLE_INSTALL_SETUID),1)
+INSTALL_SEPARATED_OPTIONS = -m4710
+progs_install_separated += $(progs_separated_fscaps)
+endif
+ifdef SETCAP_GROUP
+INSTALL_SEPARATED_OPTIONS += -g$(SETCAP_GROUP)
+endif
+
 # Parse "int cmd_xxx_yyy(int argc, char **argv)" lines in cfiles, and
 # create whitespace separated map of form: "btrfs-xxx-yyy@key:value".
 sc_cfiles := $(wildcard cmds-*.c)
@@ -704,7 +723,7 @@ $(CLEANDIRS):
 	@echo "Cleaning $(patsubst clean-%,%,$@)"
 	$(Q)$(MAKE) $(MAKEOPTS) -C $(patsubst clean-%,%,$@) clean
 
-install: $(libs_build) $(progs_install) $(INSTALLDIRS)
+install: $(libs_build) $(progs_install) $(progs_install_separated) $(INSTALLDIRS)
 ifeq ($(BUILD_PROGRAMS),1)
 	$(INSTALL) -m755 -d $(DESTDIR)$(bindir)
 	$(INSTALL) $(progs_install) $(DESTDIR)$(bindir)
@@ -726,6 +745,9 @@ endif
 	$(INSTALL) -m644 $(libbtrfs_headers) $(DESTDIR)$(incdir)/btrfs
 	$(INSTALL) -m644 libbtrfsutil/btrfsutil.h $(DESTDIR)$(incdir)
 endif
+ifeq ($(BUILD_BTRFS_SEPARATED),1)
+	$(Q)$(MAKE) $(MAKEOPTS) install-separated
+endif
 
 ifeq ($(PYTHON_BINDINGS),1)
 install_python: libbtrfsutil_python
@@ -741,6 +763,18 @@ install-static: $(progs_static) $(INSTALLDIRS)
 	# btrfsck is a link to btrfs in the src tree, make it so for installed file as well
 	$(LN_S) -f btrfs.static $(DESTDIR)$(bindir)/btrfsck.static
 
+# install separated btrfs binaries, set linux capabilities(7) defined
+# in "@SEPARATED" lines using setcap(8), remove ".separated" postfix
+install-btrfs-%.separated: btrfs-%.separated
+	$(INSTALL) -m755 -d $(DESTDIR)$(bindir)
+	$(INSTALL) $(INSTALL_SEPARATED_OPTIONS) $< $(DESTDIR)$(bindir)
+ifeq ($(ENABLE_INSTALL_SETCAP),1)
+	$(SETCAP) $(call sc_get,$(<:%.separated=%),fscaps)+ep $(DESTDIR)$(bindir)/$<
+endif
+	$(MV) $(DESTDIR)$(bindir)/$< $(DESTDIR)$(bindir)/$(<:%.separated=%)
+
+install-separated: $(progs_install_separated) $(patsubst %,install-%,$(progs_install_separated))
+
 $(INSTALLDIRS):
 	@echo "Making install in $(patsubst install-%,%,$@)"
 	$(Q)$(MAKE) $(MAKEOPTS) -C $(patsubst install-%,%,$@) install
diff --git a/Makefile.inc.in b/Makefile.inc.in
index a86c528e..93df2edf 100644
--- a/Makefile.inc.in
+++ b/Makefile.inc.in
@@ -10,6 +10,8 @@ AR = @AR@
 RM = @RM@
 RMDIR = @RMDIR@
 INSTALL = @INSTALL@
+MV = @MV@
+SETCAP = @SETCAP@
 DISABLE_DOCUMENTATION = @DISABLE_DOCUMENTATION@
 DISABLE_BTRFSCONVERT = @DISABLE_BTRFSCONVERT@
 BUILD_PROGRAMS = @BUILD_PROGRAMS@
@@ -22,6 +24,10 @@ PYTHON_BINDINGS = @PYTHON_BINDINGS@
 PYTHON = @PYTHON@
 PYTHON_CFLAGS = @PYTHON_CFLAGS@
 
+BUILD_BTRFS_SEPARATED = @BUILD_BTRFS_SEPARATED@
+ENABLE_INSTALL_SETCAP = @ENABLE_INSTALL_SETCAP@
+ENABLE_INSTALL_SETUID = @ENABLE_INSTALL_SETUID@
+
 SUBST_CFLAGS = @CFLAGS@
 SUBST_LDFLAGS = @LDFLAGS@
 
diff --git a/configure.ac b/configure.ac
index df02f206..981250bc 100644
--- a/configure.ac
+++ b/configure.ac
@@ -39,6 +39,8 @@ AC_PROG_LN_S
 AC_CHECK_TOOL([AR], [ar])
 AC_PATH_PROG([RM], [rm], [rm])
 AC_PATH_PROG([RMDIR], [rmdir], [rmdir])
+AC_PATH_PROG([MV], [mv], [mv])
+AC_PATH_PROG([SETCAP], [setcap], [setcap])
 
 
 AC_CHECK_FUNCS([openat], [],
@@ -248,6 +250,40 @@ AS_IF([test "x$enable_python" = xyes], [PYTHON_BINDINGS=1], [PYTHON_BINDINGS=0])
 AC_SUBST(PYTHON_BINDINGS)
 AC_SUBST(PYTHON)
 
+# check whether to build/install separated btrfs binaries
+AC_ARG_ENABLE([setcap-install],
+	AS_HELP_STRING([--enable-setcap-install], [install separated binaries with capabilities]),
+	[], [enable_setcap_install=no]
+)
+AS_IF([test "x$enable_setcap_install" = xyes], [ENABLE_INSTALL_SETCAP=1], [ENABLE_INSTALL_SETCAP=0])
+AC_SUBST([ENABLE_INSTALL_SETCAP])
+AM_CONDITIONAL(SETCAP_INSTALL, test x$enable_setcap_install = xyes)
+
+AC_ARG_ENABLE([setuid-install],
+	AS_HELP_STRING([--enable-setuid-install], [install separated binaries as setuid]),
+	[], [enable_setuid_install=no]
+)
+AS_IF([test "x$enable_setuid_install" = xyes], [ENABLE_INSTALL_SETUID=1], [ENABLE_INSTALL_SETUID=0])
+AC_SUBST([ENABLE_INSTALL_SETUID])
+
+if test "x$enable_setcap_install" = "xyes"; then
+	enable_btrfs_separated=yes
+	if test "x$enable_setuid_install" = "xyes"; then
+		AC_MSG_RESULT(setcap and setuid both selected)
+		AC_MSG_ERROR(You must choose either setcap-install or setuid-install)
+	fi
+fi
+if test "x$enable_setuid_install" = "xyes"; then
+	enable_btrfs_separated=yes
+fi
+
+AC_ARG_ENABLE([btrfs-separated],
+	AS_HELP_STRING([--enable-btrfs-separated], [build separated binaries for btrfs subcommands]),
+	[], [enable_btrfs_separated=no]
+)
+AS_IF([test "x$enable_btrfs_separated" = xyes], [BUILD_BTRFS_SEPARATED=1], [BUILD_BTRFS_SEPARATED=0])
+AC_SUBST([BUILD_BTRFS_SEPARATED])
+
 # udev v190 introduced the btrfs builtin and a udev rule to use it.
 # Our udev rule gives us the friendly dm names but isn't required (or valid)
 # on earlier releases.
@@ -309,5 +345,9 @@ AC_MSG_RESULT([
 	Python bindings:    ${enable_python}
 	Python interpreter: ${PYTHON}
 
+	separated progs:    ${enable_btrfs_separated}
+	Install separated progs with capabilities: ${enable_setcap_install}
+	Install separated progs suid: ${enable_setuid_install}
+
 	Type 'make' to compile.
 ])
-- 
2.16.4

Re: [RFC PATCH v2 0/4] btrfs-progs: build distinct binaries for specific btrfs subcommands

[Axel Burri]

In Reply to:

On 30/08/2018 04.38, Misono Tomohiro wrote:
>
> Hello,
>
> Not directly related this series and just FYI,
> I'm working to allow sub show/list to non-privileged user as long
> as he can access to the subvolume:
>   https://www.spinics.net/lists/linux-btrfs/msg79285.html
>
> Hopefully this will be merged to master in near future
> (any comments from user/dev is welcome).
>
> Thanks,
> Misono
>

I found some time to play around with your patchset, it worked fine on
4.18.8-gentoo kernel.

As far as I can see, only "btrfs subvolume show" and "btrfs subvolume
list" makes sense for a regular user?

With the "cmds-separated-fscaps-v2" patchset [1], you can build
separated binaries as follows:

    # make btrfs-subvolume-list.separated
    # make btrfs-subvolume-show.separated

Now not everybody wants to install these with fscaps or setuid, but it
might also make sense to provide "/usr/bin/btrfs-subvolume-{show,list}",
as they now work for a regular user. Having both root/user binaries
concurrently is not an issue (e.g. in gentoo the full-featured btrfs
command is in "/sbin/").

Last time I checked, debian installs it to "/bin/btrfs", which from my
perspective seems to be the preferred location as soon as Misonos patch
is merged.

 [1] https://github.com/digint/btrfs-progs/tree/cmds-separated-fscaps-v2

Re: [RFC PATCH v2 0/4] btrfs-progs: build distinct binaries for specific btrfs subcommands

[Duncan]

Axel Burri posted on Thu, 20 Sep 2018 00:02:22 +0200 as excerpted:

> Now not everybody wants to install these with fscaps or setuid, but it
> might also make sense to provide "/usr/bin/btrfs-subvolume-{show,list}",
> as they now work for a regular user. Having both root/user binaries
> concurrently is not an issue (e.g. in gentoo the full-featured btrfs
> command is in "/sbin/").

That's going to be a problem for distros (or users like me with advanced 
layouts, on gentoo too FWIW) that have the bin/sbin merge, where one is a 
symlink to the other.

FWIW I have both the /usr merge (tho reversed for me, so /usr -> . 
instead of having to have /bin and /sbin symlinks to /usr/bin) and the 
bin/sbin merge, along with, since I'm on amd64-nomultilib, the lib/lib64 
merge.  So:

$$ dir -gGd /bin /sbin /usr /lib /lib64
drwxr-xr-x 1 35688 Sep 18 22:56 /bin
lrwxrwxrwx 1     5 Aug  7 00:29 /lib -> lib64
drwxr-xr-x 1 78560 Sep 18 22:56 /lib64
lrwxrwxrwx 1     3 Mar 11  2018 /sbin -> bin
lrwxrwxrwx 1     1 Mar 11  2018 /usr -> .


Of course that last one (/usr -> .) leads to /share and /include hanging 
directly off of / as well, but it works.

But in that scheme /bin, /sbin, /usr/bin and /usr/sbin, are all the same 
dir, so only one executable of a particularly name can exist therein.

-- 
Duncan - List replies preferred.   No HTML msgs.
"Every nonfree program has a lord, a master --
and if you use the program, he is your master."  Richard Stallman

Re: [RFC PATCH v2 0/4] btrfs-progs: build distinct binaries for specific btrfs subcommands

[Axel Burri]

On 20/09/2018 10.32, Duncan wrote:
> Axel Burri posted on Thu, 20 Sep 2018 00:02:22 +0200 as excerpted:
> 
>> Now not everybody wants to install these with fscaps or setuid, but it
>> might also make sense to provide "/usr/bin/btrfs-subvolume-{show,list}",
>> as they now work for a regular user. Having both root/user binaries
>> concurrently is not an issue (e.g. in gentoo the full-featured btrfs
>> command is in "/sbin/").
> 
> That's going to be a problem for distros (or users like me with advanced 
> layouts, on gentoo too FWIW) that have the bin/sbin merge, where one is a 
> symlink to the other.

I think you got me wrong here: There will not be binaries with the same
filename. I totally agree that this would be a bad thing, no matter if
you have bin/sbin merged or not, you'll end up in either having a
collision or (even worse) rely on the order in $PATH.

With this "separated" patchset, you can install a binary
"btrfs-subvolume-show", which has the same functionality as "btrfs
subvolume show" (note the whitespace/dash), ending up with:

    /sbin/btrfs
    /usr/bin/btrfs-subvolume-show
    /usr/bin/btrfs-subvolume-list

thus allowing a user to run "btrfs-subvolume-show" while "btrfs" is
still only visible for root. With /usr merge it would look like this:

    /bin/btrfs
    /bin/btrfs-subvolume-show
    /bin/btrfs-subvolume-list


What I'm thinking of is a distro package which installs the separated
binaries, either with suid/fscaps (if the admin wants to provide their
users with non-restricted versions of specific commands), or without
special flags (if the admin wants to just provide the binaries that also
work for non-root user, enabled by Misono's patchset.

See my gentoo ebuild:
https://github.com/digint/gentoo/tree/btrfs-progs-separated/sys-fs/btrfs-progs-separated


> 
> FWIW I have both the /usr merge (tho reversed for me, so /usr -> . 
> instead of having to have /bin and /sbin symlinks to /usr/bin) and the 
> bin/sbin merge, along with, since I'm on amd64-nomultilib, the lib/lib64 
> merge.  So:
> 
> $$ dir -gGd /bin /sbin /usr /lib /lib64
> drwxr-xr-x 1 35688 Sep 18 22:56 /bin
> lrwxrwxrwx 1     5 Aug  7 00:29 /lib -> lib64
> drwxr-xr-x 1 78560 Sep 18 22:56 /lib64
> lrwxrwxrwx 1     3 Mar 11  2018 /sbin -> bin
> lrwxrwxrwx 1     1 Mar 11  2018 /usr -> .
> 
> 
> Of course that last one (/usr -> .) leads to /share and /include hanging 
> directly off of / as well, but it works.
> 
> But in that scheme /bin, /sbin, /usr/bin and /usr/sbin, are all the same 
> dir, so only one executable of a particularly name can exist therein.
> 

Re: [RFC PATCH v2 0/4] btrfs-progs: build distinct binaries for specific btrfs subcommands

[Duncan]

Axel Burri posted on Fri, 21 Sep 2018 11:46:37 +0200 as excerpted:

> I think you got me wrong here: There will not be binaries with the same
> filename. I totally agree that this would be a bad thing, no matter if
> you have bin/sbin merged or not, you'll end up in either having a
> collision or (even worse) rely on the order in $PATH.
> 
> With this "separated" patchset, you can install a binary
> "btrfs-subvolume-show", which has the same functionality as "btrfs
> subvolume show" (note the whitespace/dash), ending up with:
> 
>     /sbin/btrfs
>     /usr/bin/btrfs-subvolume-show
>     /usr/bin/btrfs-subvolume-list

I did get you wrong (and had even understood the separately named 
binaries from an earlier post, too, but forgot).

Thanks. =:^)

-- 
Duncan - List replies preferred.   No HTML msgs.
"Every nonfree program has a lord, a master --
and if you use the program, he is your master."  Richard Stallman