* [Virtio-fs] [PATCH 0/3] virtiofsd capability changes and addition @ 2020-06-25 16:29 ` Dr. David Alan Gilbert (git) 0 siblings, 0 replies; 16+ messages in thread From: Dr. David Alan Gilbert (git) @ 2020-06-25 16:29 UTC (permalink / raw) To: qemu-devel, virtio-fs, stefanha, vgoyal From: "Dr. David Alan Gilbert" <dgilbert@redhat.com> Hi, This is a set of changes relating to the capability restirctions introduced in virtiofsd back in a59feb483b8. The first one is a potentially important fix; the missing terminator could mean extra capabilities are added based on junk on the stack; although that's not been seen in practice. Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com> Dr. David Alan Gilbert (3): virtiofsd: Terminate capability list virtiofsd: Check capability calls virtiofsd: Allow addition or removal of capabilities docs/tools/virtiofsd.rst | 5 +++ tools/virtiofsd/helper.c | 2 + tools/virtiofsd/passthrough_ll.c | 68 +++++++++++++++++++++++++++++--- 3 files changed, 70 insertions(+), 5 deletions(-) -- 2.26.2 ^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH 0/3] virtiofsd capability changes and addition @ 2020-06-25 16:29 ` Dr. David Alan Gilbert (git) 0 siblings, 0 replies; 16+ messages in thread From: Dr. David Alan Gilbert (git) @ 2020-06-25 16:29 UTC (permalink / raw) To: qemu-devel, virtio-fs, stefanha, vgoyal From: "Dr. David Alan Gilbert" <dgilbert@redhat.com> Hi, This is a set of changes relating to the capability restirctions introduced in virtiofsd back in a59feb483b8. The first one is a potentially important fix; the missing terminator could mean extra capabilities are added based on junk on the stack; although that's not been seen in practice. Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com> Dr. David Alan Gilbert (3): virtiofsd: Terminate capability list virtiofsd: Check capability calls virtiofsd: Allow addition or removal of capabilities docs/tools/virtiofsd.rst | 5 +++ tools/virtiofsd/helper.c | 2 + tools/virtiofsd/passthrough_ll.c | 68 +++++++++++++++++++++++++++++--- 3 files changed, 70 insertions(+), 5 deletions(-) -- 2.26.2 ^ permalink raw reply [flat|nested] 16+ messages in thread
* [Virtio-fs] [PATCH 1/3] virtiofsd: Terminate capability list 2020-06-25 16:29 ` Dr. David Alan Gilbert (git) @ 2020-06-25 16:29 ` Dr. David Alan Gilbert (git) -1 siblings, 0 replies; 16+ messages in thread From: Dr. David Alan Gilbert (git) @ 2020-06-25 16:29 UTC (permalink / raw) To: qemu-devel, virtio-fs, stefanha, vgoyal From: "Dr. David Alan Gilbert" <dgilbert@redhat.com> capng_updatev is a varargs function that needs a -1 to terminate it, but it was missing. In practice what seems to have been happening is that it's added the capabilities we asked for, then runs into junk on the stack, so if we're unlucky it might be adding some more, but in reality it's failing - but after adding the capabilities we asked for. Fixes: a59feb483b8 ("virtiofsd: only retain file system capabilities") Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com> --- tools/virtiofsd/passthrough_ll.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/tools/virtiofsd/passthrough_ll.c b/tools/virtiofsd/passthrough_ll.c index 2ce7c96085..e373e3b36e 100644 --- a/tools/virtiofsd/passthrough_ll.c +++ b/tools/virtiofsd/passthrough_ll.c @@ -2598,7 +2598,9 @@ static void setup_capabilities(void) CAP_SETGID, CAP_SETUID, CAP_MKNOD, - CAP_SETFCAP); + CAP_SETFCAP, + -1); + capng_apply(CAPNG_SELECT_BOTH); cap.saved = capng_save_state(); -- 2.26.2 ^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH 1/3] virtiofsd: Terminate capability list @ 2020-06-25 16:29 ` Dr. David Alan Gilbert (git) 0 siblings, 0 replies; 16+ messages in thread From: Dr. David Alan Gilbert (git) @ 2020-06-25 16:29 UTC (permalink / raw) To: qemu-devel, virtio-fs, stefanha, vgoyal From: "Dr. David Alan Gilbert" <dgilbert@redhat.com> capng_updatev is a varargs function that needs a -1 to terminate it, but it was missing. In practice what seems to have been happening is that it's added the capabilities we asked for, then runs into junk on the stack, so if we're unlucky it might be adding some more, but in reality it's failing - but after adding the capabilities we asked for. Fixes: a59feb483b8 ("virtiofsd: only retain file system capabilities") Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com> --- tools/virtiofsd/passthrough_ll.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/tools/virtiofsd/passthrough_ll.c b/tools/virtiofsd/passthrough_ll.c index 2ce7c96085..e373e3b36e 100644 --- a/tools/virtiofsd/passthrough_ll.c +++ b/tools/virtiofsd/passthrough_ll.c @@ -2598,7 +2598,9 @@ static void setup_capabilities(void) CAP_SETGID, CAP_SETUID, CAP_MKNOD, - CAP_SETFCAP); + CAP_SETFCAP, + -1); + capng_apply(CAPNG_SELECT_BOTH); cap.saved = capng_save_state(); -- 2.26.2 ^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [Virtio-fs] [PATCH 1/3] virtiofsd: Terminate capability list 2020-06-25 16:29 ` Dr. David Alan Gilbert (git) @ 2020-06-26 10:31 ` Stefan Hajnoczi -1 siblings, 0 replies; 16+ messages in thread From: Stefan Hajnoczi @ 2020-06-26 10:31 UTC (permalink / raw) To: Dr. David Alan Gilbert (git); +Cc: virtio-fs, qemu-devel, vgoyal [-- Attachment #1: Type: text/plain, Size: 807 bytes --] On Thu, Jun 25, 2020 at 05:29:27PM +0100, Dr. David Alan Gilbert (git) wrote: > From: "Dr. David Alan Gilbert" <dgilbert@redhat.com> > > capng_updatev is a varargs function that needs a -1 to terminate it, > but it was missing. > > In practice what seems to have been happening is that it's added the > capabilities we asked for, then runs into junk on the stack, so if > we're unlucky it might be adding some more, but in reality it's > failing - but after adding the capabilities we asked for. > > Fixes: a59feb483b8 ("virtiofsd: only retain file system capabilities") > Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com> > --- > tools/virtiofsd/passthrough_ll.c | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 488 bytes --] ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 1/3] virtiofsd: Terminate capability list @ 2020-06-26 10:31 ` Stefan Hajnoczi 0 siblings, 0 replies; 16+ messages in thread From: Stefan Hajnoczi @ 2020-06-26 10:31 UTC (permalink / raw) To: Dr. David Alan Gilbert (git); +Cc: virtio-fs, qemu-devel, vgoyal [-- Attachment #1: Type: text/plain, Size: 807 bytes --] On Thu, Jun 25, 2020 at 05:29:27PM +0100, Dr. David Alan Gilbert (git) wrote: > From: "Dr. David Alan Gilbert" <dgilbert@redhat.com> > > capng_updatev is a varargs function that needs a -1 to terminate it, > but it was missing. > > In practice what seems to have been happening is that it's added the > capabilities we asked for, then runs into junk on the stack, so if > we're unlucky it might be adding some more, but in reality it's > failing - but after adding the capabilities we asked for. > > Fixes: a59feb483b8 ("virtiofsd: only retain file system capabilities") > Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com> > --- > tools/virtiofsd/passthrough_ll.c | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 488 bytes --] ^ permalink raw reply [flat|nested] 16+ messages in thread
* [Virtio-fs] [PATCH 2/3] virtiofsd: Check capability calls 2020-06-25 16:29 ` Dr. David Alan Gilbert (git) @ 2020-06-25 16:29 ` Dr. David Alan Gilbert (git) -1 siblings, 0 replies; 16+ messages in thread From: Dr. David Alan Gilbert (git) @ 2020-06-25 16:29 UTC (permalink / raw) To: qemu-devel, virtio-fs, stefanha, vgoyal From: "Dr. David Alan Gilbert" <dgilbert@redhat.com> Check the capability calls worked. Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com> --- tools/virtiofsd/passthrough_ll.c | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/tools/virtiofsd/passthrough_ll.c b/tools/virtiofsd/passthrough_ll.c index e373e3b36e..99d562046a 100644 --- a/tools/virtiofsd/passthrough_ll.c +++ b/tools/virtiofsd/passthrough_ll.c @@ -2589,7 +2589,7 @@ static void setup_capabilities(void) */ capng_setpid(syscall(SYS_gettid)); capng_clear(CAPNG_SELECT_BOTH); - capng_updatev(CAPNG_ADD, CAPNG_PERMITTED | CAPNG_EFFECTIVE, + if (capng_updatev(CAPNG_ADD, CAPNG_PERMITTED | CAPNG_EFFECTIVE, CAP_CHOWN, CAP_DAC_OVERRIDE, CAP_DAC_READ_SEARCH, @@ -2599,11 +2599,21 @@ static void setup_capabilities(void) CAP_SETUID, CAP_MKNOD, CAP_SETFCAP, - -1); + -1)) { + fuse_log(FUSE_LOG_ERR, "%s: capng_updatev failed\n", __func__); + exit(1); + } - capng_apply(CAPNG_SELECT_BOTH); + if (capng_apply(CAPNG_SELECT_BOTH)) { + fuse_log(FUSE_LOG_ERR, "%s: capng_apply failed\n", __func__); + exit(1); + } cap.saved = capng_save_state(); + if (!cap.saved) { + fuse_log(FUSE_LOG_ERR, "%s: capng_save_state failed\n", __func__); + exit(1); + } pthread_mutex_unlock(&cap.mutex); } -- 2.26.2 ^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH 2/3] virtiofsd: Check capability calls @ 2020-06-25 16:29 ` Dr. David Alan Gilbert (git) 0 siblings, 0 replies; 16+ messages in thread From: Dr. David Alan Gilbert (git) @ 2020-06-25 16:29 UTC (permalink / raw) To: qemu-devel, virtio-fs, stefanha, vgoyal From: "Dr. David Alan Gilbert" <dgilbert@redhat.com> Check the capability calls worked. Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com> --- tools/virtiofsd/passthrough_ll.c | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/tools/virtiofsd/passthrough_ll.c b/tools/virtiofsd/passthrough_ll.c index e373e3b36e..99d562046a 100644 --- a/tools/virtiofsd/passthrough_ll.c +++ b/tools/virtiofsd/passthrough_ll.c @@ -2589,7 +2589,7 @@ static void setup_capabilities(void) */ capng_setpid(syscall(SYS_gettid)); capng_clear(CAPNG_SELECT_BOTH); - capng_updatev(CAPNG_ADD, CAPNG_PERMITTED | CAPNG_EFFECTIVE, + if (capng_updatev(CAPNG_ADD, CAPNG_PERMITTED | CAPNG_EFFECTIVE, CAP_CHOWN, CAP_DAC_OVERRIDE, CAP_DAC_READ_SEARCH, @@ -2599,11 +2599,21 @@ static void setup_capabilities(void) CAP_SETUID, CAP_MKNOD, CAP_SETFCAP, - -1); + -1)) { + fuse_log(FUSE_LOG_ERR, "%s: capng_updatev failed\n", __func__); + exit(1); + } - capng_apply(CAPNG_SELECT_BOTH); + if (capng_apply(CAPNG_SELECT_BOTH)) { + fuse_log(FUSE_LOG_ERR, "%s: capng_apply failed\n", __func__); + exit(1); + } cap.saved = capng_save_state(); + if (!cap.saved) { + fuse_log(FUSE_LOG_ERR, "%s: capng_save_state failed\n", __func__); + exit(1); + } pthread_mutex_unlock(&cap.mutex); } -- 2.26.2 ^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [Virtio-fs] [PATCH 2/3] virtiofsd: Check capability calls 2020-06-25 16:29 ` Dr. David Alan Gilbert (git) @ 2020-06-26 10:31 ` Stefan Hajnoczi -1 siblings, 0 replies; 16+ messages in thread From: Stefan Hajnoczi @ 2020-06-26 10:31 UTC (permalink / raw) To: Dr. David Alan Gilbert (git); +Cc: virtio-fs, qemu-devel, vgoyal [-- Attachment #1: Type: text/plain, Size: 417 bytes --] On Thu, Jun 25, 2020 at 05:29:28PM +0100, Dr. David Alan Gilbert (git) wrote: > From: "Dr. David Alan Gilbert" <dgilbert@redhat.com> > > Check the capability calls worked. > > Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com> > --- > tools/virtiofsd/passthrough_ll.c | 16 +++++++++++++--- > 1 file changed, 13 insertions(+), 3 deletions(-) Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 488 bytes --] ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 2/3] virtiofsd: Check capability calls @ 2020-06-26 10:31 ` Stefan Hajnoczi 0 siblings, 0 replies; 16+ messages in thread From: Stefan Hajnoczi @ 2020-06-26 10:31 UTC (permalink / raw) To: Dr. David Alan Gilbert (git); +Cc: virtio-fs, qemu-devel, vgoyal [-- Attachment #1: Type: text/plain, Size: 417 bytes --] On Thu, Jun 25, 2020 at 05:29:28PM +0100, Dr. David Alan Gilbert (git) wrote: > From: "Dr. David Alan Gilbert" <dgilbert@redhat.com> > > Check the capability calls worked. > > Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com> > --- > tools/virtiofsd/passthrough_ll.c | 16 +++++++++++++--- > 1 file changed, 13 insertions(+), 3 deletions(-) Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 488 bytes --] ^ permalink raw reply [flat|nested] 16+ messages in thread
* [Virtio-fs] [PATCH 3/3] virtiofsd: Allow addition or removal of capabilities 2020-06-25 16:29 ` Dr. David Alan Gilbert (git) @ 2020-06-25 16:29 ` Dr. David Alan Gilbert (git) -1 siblings, 0 replies; 16+ messages in thread From: Dr. David Alan Gilbert (git) @ 2020-06-25 16:29 UTC (permalink / raw) To: qemu-devel, virtio-fs, stefanha, vgoyal From: "Dr. David Alan Gilbert" <dgilbert@redhat.com> Allow capabilities to be added or removed from the allowed set for the daemon; e.g. default: CapPrm: 00000000880000df CapEff: 00000000880000df -o modcaps=+sys_admin CapPrm: 00000000882000df CapEff: 00000000882000df -o modcaps=+sys_admin:-chown CapPrm: 00000000882000de CapEff: 00000000882000de Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com> --- docs/tools/virtiofsd.rst | 5 ++++ tools/virtiofsd/helper.c | 2 ++ tools/virtiofsd/passthrough_ll.c | 50 ++++++++++++++++++++++++++++++-- 3 files changed, 55 insertions(+), 2 deletions(-) diff --git a/docs/tools/virtiofsd.rst b/docs/tools/virtiofsd.rst index 378594c422..824e713491 100644 --- a/docs/tools/virtiofsd.rst +++ b/docs/tools/virtiofsd.rst @@ -54,6 +54,11 @@ Options * flock|no_flock - Enable/disable flock. The default is ``no_flock``. + * modcaps=CAPLIST + Modify the list of capabilities allowed; CAPLIST is a colon separated + list of capabilities, each preceded by either + or -, e.g. + ''+sys_admin:-chown''. + * log_level=LEVEL - Print only log messages matching LEVEL or more severe. LEVEL is one of ``err``, ``warn``, ``info``, or ``debug``. The default is ``info``. diff --git a/tools/virtiofsd/helper.c b/tools/virtiofsd/helper.c index 00a1ef666a..3105b6c23a 100644 --- a/tools/virtiofsd/helper.c +++ b/tools/virtiofsd/helper.c @@ -174,6 +174,8 @@ void fuse_cmdline_help(void) " default: no_writeback\n" " -o xattr|no_xattr enable/disable xattr\n" " default: no_xattr\n" + " -o modcaps=CAPLIST Modify the list of capabilities\n" + " e.g. -o modcaps=+sys_admin:-chown\n" " --rlimit-nofile=<num> set maximum number of file descriptors\n" " (0 leaves rlimit unchanged)\n" " default: min(1000000, fs.file-max - 16384)\n" diff --git a/tools/virtiofsd/passthrough_ll.c b/tools/virtiofsd/passthrough_ll.c index 99d562046a..9d2cbc70ca 100644 --- a/tools/virtiofsd/passthrough_ll.c +++ b/tools/virtiofsd/passthrough_ll.c @@ -145,6 +145,7 @@ struct lo_data { int posix_lock; int xattr; char *source; + char *modcaps; double timeout; int cache; int timeout_set; @@ -170,6 +171,7 @@ static const struct fuse_opt lo_opts[] = { { "no_posix_lock", offsetof(struct lo_data, posix_lock), 0 }, { "xattr", offsetof(struct lo_data, xattr), 1 }, { "no_xattr", offsetof(struct lo_data, xattr), 0 }, + { "modcaps=%s", offsetof(struct lo_data, modcaps), 0 }, { "timeout=%lf", offsetof(struct lo_data, timeout), 0 }, { "timeout=", offsetof(struct lo_data, timeout_set), 1 }, { "cache=none", offsetof(struct lo_data, cache), CACHE_NONE }, @@ -2571,7 +2573,7 @@ static void setup_mounts(const char *source) /* * Only keep whitelisted capabilities that are needed for file system operation */ -static void setup_capabilities(void) +static void setup_capabilities(struct lo_data *lo) { pthread_mutex_lock(&cap.mutex); capng_restore_state(&cap.saved); @@ -2604,6 +2606,50 @@ static void setup_capabilities(void) exit(1); } + /* + * The modcaps option is a colon separated list of caps, + * each preceded by either + or -. + */ + while (lo->modcaps) { + capng_act_t action; + int cap; + + char *next = strchr(lo->modcaps, ':'); + if (next) { + *next = '\0'; + next++; + } + + switch (lo->modcaps[0]) { + case '+': + action = CAPNG_ADD; + break; + + case '-': + action = CAPNG_DROP; + break; + + default: + fuse_log(FUSE_LOG_ERR, + "%s: Expecting '+'/'-' in modcaps but found '%c'\n", + __func__, lo->modcaps[0]); + exit(1); + } + cap = capng_name_to_capability(lo->modcaps + 1); + if (cap < 0) { + fuse_log(FUSE_LOG_ERR, "%s: Unknown capability '%s'\n", __func__, + lo->modcaps); + exit(1); + } + if (capng_update(action, CAPNG_PERMITTED | CAPNG_EFFECTIVE, cap)) { + fuse_log(FUSE_LOG_ERR, "%s: capng_update failed for '%s'\n", + __func__, lo->modcaps); + exit(1); + } + + lo->modcaps = next; + } + if (capng_apply(CAPNG_SELECT_BOTH)) { fuse_log(FUSE_LOG_ERR, "%s: capng_apply failed\n", __func__); exit(1); @@ -2627,7 +2673,7 @@ static void setup_sandbox(struct lo_data *lo, struct fuse_session *se, setup_namespaces(lo, se); setup_mounts(lo->source); setup_seccomp(enable_syslog); - setup_capabilities(); + setup_capabilities(lo); } /* Set the maximum number of open file descriptors */ -- 2.26.2 ^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH 3/3] virtiofsd: Allow addition or removal of capabilities @ 2020-06-25 16:29 ` Dr. David Alan Gilbert (git) 0 siblings, 0 replies; 16+ messages in thread From: Dr. David Alan Gilbert (git) @ 2020-06-25 16:29 UTC (permalink / raw) To: qemu-devel, virtio-fs, stefanha, vgoyal From: "Dr. David Alan Gilbert" <dgilbert@redhat.com> Allow capabilities to be added or removed from the allowed set for the daemon; e.g. default: CapPrm: 00000000880000df CapEff: 00000000880000df -o modcaps=+sys_admin CapPrm: 00000000882000df CapEff: 00000000882000df -o modcaps=+sys_admin:-chown CapPrm: 00000000882000de CapEff: 00000000882000de Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com> --- docs/tools/virtiofsd.rst | 5 ++++ tools/virtiofsd/helper.c | 2 ++ tools/virtiofsd/passthrough_ll.c | 50 ++++++++++++++++++++++++++++++-- 3 files changed, 55 insertions(+), 2 deletions(-) diff --git a/docs/tools/virtiofsd.rst b/docs/tools/virtiofsd.rst index 378594c422..824e713491 100644 --- a/docs/tools/virtiofsd.rst +++ b/docs/tools/virtiofsd.rst @@ -54,6 +54,11 @@ Options * flock|no_flock - Enable/disable flock. The default is ``no_flock``. + * modcaps=CAPLIST + Modify the list of capabilities allowed; CAPLIST is a colon separated + list of capabilities, each preceded by either + or -, e.g. + ''+sys_admin:-chown''. + * log_level=LEVEL - Print only log messages matching LEVEL or more severe. LEVEL is one of ``err``, ``warn``, ``info``, or ``debug``. The default is ``info``. diff --git a/tools/virtiofsd/helper.c b/tools/virtiofsd/helper.c index 00a1ef666a..3105b6c23a 100644 --- a/tools/virtiofsd/helper.c +++ b/tools/virtiofsd/helper.c @@ -174,6 +174,8 @@ void fuse_cmdline_help(void) " default: no_writeback\n" " -o xattr|no_xattr enable/disable xattr\n" " default: no_xattr\n" + " -o modcaps=CAPLIST Modify the list of capabilities\n" + " e.g. -o modcaps=+sys_admin:-chown\n" " --rlimit-nofile=<num> set maximum number of file descriptors\n" " (0 leaves rlimit unchanged)\n" " default: min(1000000, fs.file-max - 16384)\n" diff --git a/tools/virtiofsd/passthrough_ll.c b/tools/virtiofsd/passthrough_ll.c index 99d562046a..9d2cbc70ca 100644 --- a/tools/virtiofsd/passthrough_ll.c +++ b/tools/virtiofsd/passthrough_ll.c @@ -145,6 +145,7 @@ struct lo_data { int posix_lock; int xattr; char *source; + char *modcaps; double timeout; int cache; int timeout_set; @@ -170,6 +171,7 @@ static const struct fuse_opt lo_opts[] = { { "no_posix_lock", offsetof(struct lo_data, posix_lock), 0 }, { "xattr", offsetof(struct lo_data, xattr), 1 }, { "no_xattr", offsetof(struct lo_data, xattr), 0 }, + { "modcaps=%s", offsetof(struct lo_data, modcaps), 0 }, { "timeout=%lf", offsetof(struct lo_data, timeout), 0 }, { "timeout=", offsetof(struct lo_data, timeout_set), 1 }, { "cache=none", offsetof(struct lo_data, cache), CACHE_NONE }, @@ -2571,7 +2573,7 @@ static void setup_mounts(const char *source) /* * Only keep whitelisted capabilities that are needed for file system operation */ -static void setup_capabilities(void) +static void setup_capabilities(struct lo_data *lo) { pthread_mutex_lock(&cap.mutex); capng_restore_state(&cap.saved); @@ -2604,6 +2606,50 @@ static void setup_capabilities(void) exit(1); } + /* + * The modcaps option is a colon separated list of caps, + * each preceded by either + or -. + */ + while (lo->modcaps) { + capng_act_t action; + int cap; + + char *next = strchr(lo->modcaps, ':'); + if (next) { + *next = '\0'; + next++; + } + + switch (lo->modcaps[0]) { + case '+': + action = CAPNG_ADD; + break; + + case '-': + action = CAPNG_DROP; + break; + + default: + fuse_log(FUSE_LOG_ERR, + "%s: Expecting '+'/'-' in modcaps but found '%c'\n", + __func__, lo->modcaps[0]); + exit(1); + } + cap = capng_name_to_capability(lo->modcaps + 1); + if (cap < 0) { + fuse_log(FUSE_LOG_ERR, "%s: Unknown capability '%s'\n", __func__, + lo->modcaps); + exit(1); + } + if (capng_update(action, CAPNG_PERMITTED | CAPNG_EFFECTIVE, cap)) { + fuse_log(FUSE_LOG_ERR, "%s: capng_update failed for '%s'\n", + __func__, lo->modcaps); + exit(1); + } + + lo->modcaps = next; + } + if (capng_apply(CAPNG_SELECT_BOTH)) { fuse_log(FUSE_LOG_ERR, "%s: capng_apply failed\n", __func__); exit(1); @@ -2627,7 +2673,7 @@ static void setup_sandbox(struct lo_data *lo, struct fuse_session *se, setup_namespaces(lo, se); setup_mounts(lo->source); setup_seccomp(enable_syslog); - setup_capabilities(); + setup_capabilities(lo); } /* Set the maximum number of open file descriptors */ -- 2.26.2 ^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [Virtio-fs] [PATCH 3/3] virtiofsd: Allow addition or removal of capabilities 2020-06-25 16:29 ` Dr. David Alan Gilbert (git) @ 2020-06-26 10:31 ` Stefan Hajnoczi -1 siblings, 0 replies; 16+ messages in thread From: Stefan Hajnoczi @ 2020-06-26 10:31 UTC (permalink / raw) To: Dr. David Alan Gilbert (git); +Cc: virtio-fs, qemu-devel, vgoyal [-- Attachment #1: Type: text/plain, Size: 1582 bytes --] On Thu, Jun 25, 2020 at 05:29:29PM +0100, Dr. David Alan Gilbert (git) wrote: > + /* > + * The modcaps option is a colon separated list of caps, > + * each preceded by either + or -. > + */ > + while (lo->modcaps) { > + capng_act_t action; > + int cap; > + > + char *next = strchr(lo->modcaps, ':'); > + if (next) { > + *next = '\0'; > + next++; > + } > + > + switch (lo->modcaps[0]) { > + case '+': > + action = CAPNG_ADD; > + break; > + > + case '-': > + action = CAPNG_DROP; > + break; > + > + default: > + fuse_log(FUSE_LOG_ERR, > + "%s: Expecting '+'/'-' in modcaps but found '%c'\n", > + __func__, lo->modcaps[0]); > + exit(1); > + } > + cap = capng_name_to_capability(lo->modcaps + 1); > + if (cap < 0) { > + fuse_log(FUSE_LOG_ERR, "%s: Unknown capability '%s'\n", __func__, > + lo->modcaps); > + exit(1); > + } > + if (capng_update(action, CAPNG_PERMITTED | CAPNG_EFFECTIVE, cap)) { > + fuse_log(FUSE_LOG_ERR, "%s: capng_update failed for '%s'\n", > + __func__, lo->modcaps); > + exit(1); > + } > + > + lo->modcaps = next; How about passing char *modcaps into this function so that lo->modcaps isn't modified by the parsing loop? That seems a bit cleaner and if we ever decide to free lo->modcaps it will work as expected. Stefan [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 488 bytes --] ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 3/3] virtiofsd: Allow addition or removal of capabilities @ 2020-06-26 10:31 ` Stefan Hajnoczi 0 siblings, 0 replies; 16+ messages in thread From: Stefan Hajnoczi @ 2020-06-26 10:31 UTC (permalink / raw) To: Dr. David Alan Gilbert (git); +Cc: virtio-fs, qemu-devel, vgoyal [-- Attachment #1: Type: text/plain, Size: 1582 bytes --] On Thu, Jun 25, 2020 at 05:29:29PM +0100, Dr. David Alan Gilbert (git) wrote: > + /* > + * The modcaps option is a colon separated list of caps, > + * each preceded by either + or -. > + */ > + while (lo->modcaps) { > + capng_act_t action; > + int cap; > + > + char *next = strchr(lo->modcaps, ':'); > + if (next) { > + *next = '\0'; > + next++; > + } > + > + switch (lo->modcaps[0]) { > + case '+': > + action = CAPNG_ADD; > + break; > + > + case '-': > + action = CAPNG_DROP; > + break; > + > + default: > + fuse_log(FUSE_LOG_ERR, > + "%s: Expecting '+'/'-' in modcaps but found '%c'\n", > + __func__, lo->modcaps[0]); > + exit(1); > + } > + cap = capng_name_to_capability(lo->modcaps + 1); > + if (cap < 0) { > + fuse_log(FUSE_LOG_ERR, "%s: Unknown capability '%s'\n", __func__, > + lo->modcaps); > + exit(1); > + } > + if (capng_update(action, CAPNG_PERMITTED | CAPNG_EFFECTIVE, cap)) { > + fuse_log(FUSE_LOG_ERR, "%s: capng_update failed for '%s'\n", > + __func__, lo->modcaps); > + exit(1); > + } > + > + lo->modcaps = next; How about passing char *modcaps into this function so that lo->modcaps isn't modified by the parsing loop? That seems a bit cleaner and if we ever decide to free lo->modcaps it will work as expected. Stefan [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 488 bytes --] ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [Virtio-fs] [PATCH 3/3] virtiofsd: Allow addition or removal of capabilities 2020-06-26 10:31 ` Stefan Hajnoczi @ 2020-06-26 18:42 ` Dr. David Alan Gilbert -1 siblings, 0 replies; 16+ messages in thread From: Dr. David Alan Gilbert @ 2020-06-26 18:42 UTC (permalink / raw) To: Stefan Hajnoczi; +Cc: virtio-fs, qemu-devel, vgoyal * Stefan Hajnoczi (stefanha@redhat.com) wrote: > On Thu, Jun 25, 2020 at 05:29:29PM +0100, Dr. David Alan Gilbert (git) wrote: > > + /* > > + * The modcaps option is a colon separated list of caps, > > + * each preceded by either + or -. > > + */ > > + while (lo->modcaps) { > > + capng_act_t action; > > + int cap; > > + > > + char *next = strchr(lo->modcaps, ':'); > > + if (next) { > > + *next = '\0'; > > + next++; > > + } > > + > > + switch (lo->modcaps[0]) { > > + case '+': > > + action = CAPNG_ADD; > > + break; > > + > > + case '-': > > + action = CAPNG_DROP; > > + break; > > + > > + default: > > + fuse_log(FUSE_LOG_ERR, > > + "%s: Expecting '+'/'-' in modcaps but found '%c'\n", > > + __func__, lo->modcaps[0]); > > + exit(1); > > + } > > + cap = capng_name_to_capability(lo->modcaps + 1); > > + if (cap < 0) { > > + fuse_log(FUSE_LOG_ERR, "%s: Unknown capability '%s'\n", __func__, > > + lo->modcaps); > > + exit(1); > > + } > > + if (capng_update(action, CAPNG_PERMITTED | CAPNG_EFFECTIVE, cap)) { > > + fuse_log(FUSE_LOG_ERR, "%s: capng_update failed for '%s'\n", > > + __func__, lo->modcaps); > > + exit(1); > > + } > > + > > + lo->modcaps = next; > > How about passing char *modcaps into this function so that lo->modcaps > isn't modified by the parsing loop? That seems a bit cleaner and if we > ever decide to free lo->modcaps it will work as expected. Yep, can do. Dave > Stefan -- Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 3/3] virtiofsd: Allow addition or removal of capabilities @ 2020-06-26 18:42 ` Dr. David Alan Gilbert 0 siblings, 0 replies; 16+ messages in thread From: Dr. David Alan Gilbert @ 2020-06-26 18:42 UTC (permalink / raw) To: Stefan Hajnoczi; +Cc: virtio-fs, qemu-devel, vgoyal * Stefan Hajnoczi (stefanha@redhat.com) wrote: > On Thu, Jun 25, 2020 at 05:29:29PM +0100, Dr. David Alan Gilbert (git) wrote: > > + /* > > + * The modcaps option is a colon separated list of caps, > > + * each preceded by either + or -. > > + */ > > + while (lo->modcaps) { > > + capng_act_t action; > > + int cap; > > + > > + char *next = strchr(lo->modcaps, ':'); > > + if (next) { > > + *next = '\0'; > > + next++; > > + } > > + > > + switch (lo->modcaps[0]) { > > + case '+': > > + action = CAPNG_ADD; > > + break; > > + > > + case '-': > > + action = CAPNG_DROP; > > + break; > > + > > + default: > > + fuse_log(FUSE_LOG_ERR, > > + "%s: Expecting '+'/'-' in modcaps but found '%c'\n", > > + __func__, lo->modcaps[0]); > > + exit(1); > > + } > > + cap = capng_name_to_capability(lo->modcaps + 1); > > + if (cap < 0) { > > + fuse_log(FUSE_LOG_ERR, "%s: Unknown capability '%s'\n", __func__, > > + lo->modcaps); > > + exit(1); > > + } > > + if (capng_update(action, CAPNG_PERMITTED | CAPNG_EFFECTIVE, cap)) { > > + fuse_log(FUSE_LOG_ERR, "%s: capng_update failed for '%s'\n", > > + __func__, lo->modcaps); > > + exit(1); > > + } > > + > > + lo->modcaps = next; > > How about passing char *modcaps into this function so that lo->modcaps > isn't modified by the parsing loop? That seems a bit cleaner and if we > ever decide to free lo->modcaps it will work as expected. Yep, can do. Dave > Stefan -- Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK ^ permalink raw reply [flat|nested] 16+ messages in thread
end of thread, other threads:[~2020-06-26 18:43 UTC | newest] Thread overview: 16+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2020-06-25 16:29 [Virtio-fs] [PATCH 0/3] virtiofsd capability changes and addition Dr. David Alan Gilbert (git) 2020-06-25 16:29 ` Dr. David Alan Gilbert (git) 2020-06-25 16:29 ` [Virtio-fs] [PATCH 1/3] virtiofsd: Terminate capability list Dr. David Alan Gilbert (git) 2020-06-25 16:29 ` Dr. David Alan Gilbert (git) 2020-06-26 10:31 ` [Virtio-fs] " Stefan Hajnoczi 2020-06-26 10:31 ` Stefan Hajnoczi 2020-06-25 16:29 ` [Virtio-fs] [PATCH 2/3] virtiofsd: Check capability calls Dr. David Alan Gilbert (git) 2020-06-25 16:29 ` Dr. David Alan Gilbert (git) 2020-06-26 10:31 ` [Virtio-fs] " Stefan Hajnoczi 2020-06-26 10:31 ` Stefan Hajnoczi 2020-06-25 16:29 ` [Virtio-fs] [PATCH 3/3] virtiofsd: Allow addition or removal of capabilities Dr. David Alan Gilbert (git) 2020-06-25 16:29 ` Dr. David Alan Gilbert (git) 2020-06-26 10:31 ` [Virtio-fs] " Stefan Hajnoczi 2020-06-26 10:31 ` Stefan Hajnoczi 2020-06-26 18:42 ` [Virtio-fs] " Dr. David Alan Gilbert 2020-06-26 18:42 ` Dr. David Alan Gilbert
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.