[RFC PATCH 5 4/7] nfs - cache_lib use service thread if not executing in init namespace
From: Ian Kent If pipefs is registered within a container pipefs requests should be run within their originating container also. To do that get a token to a service thread created within the container environment for usermode helper calls. Signed-off-by: Ian Kent Cc: Benjamin Coddington Cc: Al Viro Cc: J. Bruce Fields Cc: David Howells Cc: Trond Myklebust Cc: Oleg Nesterov Cc: Eric W. Biederman Cc: Jeff Layton --- fs/nfs/cache_lib.c |7 ++- include/linux/sunrpc/cache.h |2 ++ net/sunrpc/cache.c |5 + 3 files changed, 13 insertions(+), 1 deletion(-) diff --git a/fs/nfs/cache_lib.c b/fs/nfs/cache_lib.c index 5f7b053..78f9b6e 100644 --- a/fs/nfs/cache_lib.c +++ b/fs/nfs/cache_lib.c @@ -48,7 +48,12 @@ int nfs_cache_upcall(struct cache_detail *cd, char *entry_name) if (nfs_cache_getent_prog[0] == '\0') goto out; - ret = call_usermodehelper(argv[0], argv, envp, UMH_WAIT_EXEC); + if (cd->u.pipefs.umh_token) { + int token = cd->u.pipefs.umh_token; + ret = call_usermodehelper_service(argv[0], argv, envp, + token, UMH_WAIT_EXEC); + } else + ret = call_usermodehelper(argv[0], argv, envp, UMH_WAIT_EXEC); /* * Disable the upcall mechanism if we're getting an ENOENT or * EACCES error. The admin can re-enable it on the fly by using diff --git a/include/linux/sunrpc/cache.h b/include/linux/sunrpc/cache.h index 437ddb6..bb57c7e 100644 --- a/include/linux/sunrpc/cache.h +++ b/include/linux/sunrpc/cache.h @@ -68,6 +68,8 @@ struct cache_detail_procfs { struct cache_detail_pipefs { struct dentry *dir; + /* Namespace token */ + int umh_token; }; struct cache_detail { diff --git a/net/sunrpc/cache.c b/net/sunrpc/cache.c index 5199bb1..eabc0d0 100644 --- a/net/sunrpc/cache.c +++ b/net/sunrpc/cache.c @@ -1811,6 +1811,9 @@ int sunrpc_cache_register_pipefs(struct dentry *parent, if (IS_ERR(dir)) return PTR_ERR(dir); cd->u.pipefs.dir = dir; + if (cd->net != &init_net) + cd->u.pipefs.umh_token = +umh_wq_get_token(cd->u.pipefs.umh_token, "pipefs"); return 0; } EXPORT_SYMBOL_GPL(sunrpc_cache_register_pipefs); @@ -1819,6 +1822,8 @@ void sunrpc_cache_unregister_pipefs(struct cache_detail *cd) { rpc_remove_cache_dir(cd->u.pipefs.dir); cd->u.pipefs.dir = NULL; + umh_wq_put_token(cd->u.pipefs.umh_token); + cd->u.pipefs.umh_token = 0; } EXPORT_SYMBOL_GPL(sunrpc_cache_unregister_pipefs); -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[RFC PATCH 5 6/7] KEYS - use correct memory allocation flag in call_usermodehelper_keys()
From: Ian Kent When call_usermodehelper_keys() is called it assumes it won't be called with the flag UMH_NO_WAIT. Currently that's always the case. Change this to check the flag and use the correct kernel memory allocation flag to guard against future changes. Signed-off-by: Ian Kent Cc: Benjamin Coddington Cc: Al Viro Cc: J. Bruce Fields Cc: David Howells Cc: Trond Myklebust Cc: Oleg Nesterov Cc: Eric W. Biederman Cc: Jeff Layton --- security/keys/request_key.c |4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/security/keys/request_key.c b/security/keys/request_key.c index 486ef6f..e865f9f 100644 --- a/security/keys/request_key.c +++ b/security/keys/request_key.c @@ -76,8 +76,10 @@ static int call_usermodehelper_keys(char *path, char **argv, char **envp, struct key *session_keyring, int wait) { struct subprocess_info *info; + unsigned int gfp_mask = (wait & UMH_NO_WAIT) ? + GFP_ATOMIC : GFP_KERNEL; - info = call_usermodehelper_setup(path, argv, envp, GFP_KERNEL, + info = call_usermodehelper_setup(path, argv, envp, gfp_mask, umh_keys_init, umh_keys_cleanup, session_keyring); if (!info) -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[RFC PATCH 5 7/7] KEYS: exec request key within service thread of key creator
From: Ian Kent Containerized request key helper callbacks need the ability to execute a binary in a container's context. To do that get a token to a service thread created within the container environment for usermode helper calls. Signed-off-by: Ian Kent Cc: Benjamin Coddington Cc: Al Viro Cc: J. Bruce Fields Cc: David Howells Cc: Trond Myklebust Cc: Oleg Nesterov Cc: Eric W. Biederman Cc: Jeff Layton --- include/linux/key.h |3 +++ security/keys/gc.c |2 ++ security/keys/key.c |5 + security/keys/request_key.c | 34 +++--- 4 files changed, 37 insertions(+), 7 deletions(-) diff --git a/include/linux/key.h b/include/linux/key.h index e1d4715..144727d 100644 --- a/include/linux/key.h +++ b/include/linux/key.h @@ -209,6 +209,9 @@ struct key { } payload; struct assoc_array keys; }; + + /* Namespace token */ + int umh_token; }; extern struct key *key_alloc(struct key_type *type, diff --git a/security/keys/gc.c b/security/keys/gc.c index c795237..c689ffd 100644 --- a/security/keys/gc.c +++ b/security/keys/gc.c @@ -156,6 +156,8 @@ static noinline void key_gc_unused_keys(struct list_head *keys) kfree(key->description); + umh_wq_put_token(key->umh_token); + #ifdef KEY_DEBUGGING key->magic = KEY_DEBUG_MAGIC_X; #endif diff --git a/security/keys/key.c b/security/keys/key.c index aee2ec5..3ca0825 100644 --- a/security/keys/key.c +++ b/security/keys/key.c @@ -18,6 +18,8 @@ #include #include #include +#include +#include #include "internal.h" struct kmem_cache *key_jar; @@ -309,6 +311,9 @@ struct key *key_alloc(struct key_type *type, const char *desc, /* publish the key by giving it a serial number */ atomic_inc(&user->nkeys); key_alloc_serial(key); + /* If running within a container use the container namespace */ + if (current->nsproxy->net_ns != &init_net) + key->umh_token = umh_wq_get_token(0, "keys"); error: return key; diff --git a/security/keys/request_key.c b/security/keys/request_key.c index e865f9f..233e837 100644 --- a/security/keys/request_key.c +++ b/security/keys/request_key.c @@ -70,26 +70,40 @@ static void umh_keys_cleanup(struct subprocess_info *info) } /* - * Call a usermode helper with a specific session keyring. + * Call a usermode helper with a specific session keyring and execute + * within a namespace. */ -static int call_usermodehelper_keys(char *path, char **argv, char **envp, - struct key *session_keyring, int wait) +static int call_usermodehelper_keys_service(char *path, + char **argv, char **envp, + struct key *session_keyring, + int token, unsigned int wait) { struct subprocess_info *info; unsigned int gfp_mask = (wait & UMH_NO_WAIT) ? GFP_ATOMIC : GFP_KERNEL; info = call_usermodehelper_setup(path, argv, envp, gfp_mask, - umh_keys_init, umh_keys_cleanup, - session_keyring); +umh_keys_init, umh_keys_cleanup, +session_keyring); if (!info) return -ENOMEM; + info->wq_token = token; key_get(session_keyring); return call_usermodehelper_exec(info, wait); } /* + * Call a usermode helper with a specific session keyring. + */ +static int call_usermodehelper_keys(char *path, char **argv, char **envp, + struct key *session_keyring, int wait) +{ + return call_usermodehelper_keys_service(path, argv, envp, + session_keyring, 0, wait); +} + +/* * Request userspace finish the construction of a key * - execute "/sbin/request-key " */ @@ -174,8 +188,14 @@ static int call_sbin_request_key(struct key_construction *cons, argv[i] = NULL; /* do it */ - ret = call_usermodehelper_keys(argv[0], argv, envp, keyring, - UMH_WAIT_PROC); + /* If running within a container use the container namespace */ + if (key->umh_token) + ret = call_usermodehelper_keys_service(argv[0], argv, envp, + keyring, key->umh_token, + UMH_WAIT_PROC); + else + ret = call_usermodehelper_keys(argv[0], argv, envp, + keyring, UMH_WAIT_PROC); kdebug("usermode -> 0x%x", ret); if (ret >=
[RFC PATCH 5 5/7] nfs - objlayout use service thread if not executing in init namespace
From: Ian Kent If the caller is running within a container then execute the usermode helper callback within the container also. Signed-off-by: Ian Kent Cc: Benjamin Coddington Cc: Al Viro Cc: J. Bruce Fields Cc: David Howells Cc: Trond Myklebust Cc: Oleg Nesterov Cc: Eric W. Biederman Cc: Jeff Layton --- fs/nfs/objlayout/objlayout.c | 14 +- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/fs/nfs/objlayout/objlayout.c b/fs/nfs/objlayout/objlayout.c index 919efd4..1d4fd11 100644 --- a/fs/nfs/objlayout/objlayout.c +++ b/fs/nfs/objlayout/objlayout.c @@ -600,8 +600,14 @@ static int __objlayout_upcall(struct __auto_login *login) NULL }; char *argv[8]; + int umh_token; int ret; + /* If running within a container use the container namespace */ + umh_token = 0; + if (current->nsproxy->net_ns != &init_net) + umh_token = umh_wq_get_token(0, "objlayout"); + if (unlikely(!osd_login_prog[0])) { dprintk("%s: osd_login_prog is disabled\n", __func__); return -EACCES; @@ -620,7 +626,13 @@ static int __objlayout_upcall(struct __auto_login *login) argv[6] = login->systemid_hex; argv[7] = NULL; - ret = call_usermodehelper(argv[0], argv, envp, UMH_WAIT_PROC); + if (!umh_token) + ret = call_usermodehelper(argv[0], argv, envp, UMH_WAIT_PROC); + else { + ret = call_usermodehelper_service(argv[0], argv, envp, + umh_token, UMH_WAIT_PROC); + umh_wq_put_token(umh_token); + } /* * Disable the upcall mechanism if we're getting an ENOENT or * EACCES error. The admin can re-enable it on the fly by using -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[RFC PATCH 5 3/7] nfsd - use service thread if not executing in init namespace
From: Ian Kent If nfsd is running within a container the client tracking operations should run within their originating container also. To do that get a token to a service thread created within the container environment for usermode helper calls. Signed-off-by: Ian Kent Cc: Benjamin Coddington Cc: Al Viro Cc: J. Bruce Fields Cc: David Howells Cc: Trond Myklebust Cc: Oleg Nesterov Cc: Eric W. Biederman Cc: Jeff Layton --- fs/nfsd/netns.h |3 +++ fs/nfsd/nfs4recover.c | 48 +++- fs/nfsd/nfsctl.c |6 ++ 3 files changed, 40 insertions(+), 17 deletions(-) diff --git a/fs/nfsd/netns.h b/fs/nfsd/netns.h index ea6749a..099a3c5 100644 --- a/fs/nfsd/netns.h +++ b/fs/nfsd/netns.h @@ -112,6 +112,9 @@ struct nfsd_net { u32 clientid_counter; struct svc_serv *nfsd_serv; + + /* Namespace token */ + int umh_token; }; /* Simple check to find out if a given net was properly initialized */ diff --git a/fs/nfsd/nfs4recover.c b/fs/nfsd/nfs4recover.c index 1c307f0..2547edb 100644 --- a/fs/nfsd/nfs4recover.c +++ b/fs/nfsd/nfs4recover.c @@ -1184,7 +1184,8 @@ nfsd4_cltrack_grace_start(time_t grace_start) } static int -nfsd4_umh_cltrack_upcall(char *cmd, char *arg, char *env0, char *env1) +nfsd4_umh_cltrack_upcall(char *cmd, char *arg, +char *env0, char *env1, int token) { char *envp[3]; char *argv[4]; @@ -1209,7 +1210,11 @@ nfsd4_umh_cltrack_upcall(char *cmd, char *arg, char *env0, char *env1) argv[2] = arg; argv[3] = NULL; - ret = call_usermodehelper(argv[0], argv, envp, UMH_WAIT_PROC); + if (token > 0) + ret = call_usermodehelper_service(argv[0], argv, envp, + token, UMH_WAIT_PROC); + else + ret = call_usermodehelper(argv[0], argv, envp, UMH_WAIT_PROC); /* * Disable the upcall mechanism if we're getting an ENOENT or EACCES * error. The admin can re-enable it on the fly by using sysfs @@ -1252,14 +1257,8 @@ nfsd4_umh_cltrack_init(struct net *net) struct nfsd_net *nn = net_generic(net, nfsd_net_id); char *grace_start = nfsd4_cltrack_grace_start(nn->boot_time); - /* XXX: The usermode helper s not working in container yet. */ - if (net != &init_net) { - WARN(1, KERN_ERR "NFSD: attempt to initialize umh client " - "tracking in a container!\n"); - return -EINVAL; - } - - ret = nfsd4_umh_cltrack_upcall("init", NULL, grace_start, NULL); + ret = nfsd4_umh_cltrack_upcall("init", NULL, + grace_start, NULL, nn->umh_token); kfree(grace_start); return ret; } @@ -1285,6 +1284,7 @@ nfsd4_umh_cltrack_create(struct nfs4_client *clp) { char *hexid, *has_session, *grace_start; struct nfsd_net *nn = net_generic(clp->net, nfsd_net_id); + int ret; /* * With v4.0 clients, there's little difference in outcome between a @@ -1312,7 +1312,10 @@ nfsd4_umh_cltrack_create(struct nfs4_client *clp) grace_start = nfsd4_cltrack_grace_start(nn->boot_time); nfsd4_cltrack_upcall_lock(clp); - if (!nfsd4_umh_cltrack_upcall("create", hexid, has_session, grace_start)) + ret = nfsd4_umh_cltrack_upcall("create", + hexid, has_session, grace_start, + nn->umh_token); + if (!ret) set_bit(NFSD4_CLIENT_STABLE, &clp->cl_flags); nfsd4_cltrack_upcall_unlock(clp); @@ -1324,7 +1327,9 @@ nfsd4_umh_cltrack_create(struct nfs4_client *clp) static void nfsd4_umh_cltrack_remove(struct nfs4_client *clp) { + struct nfsd_net *nn = net_generic(clp->net, nfsd_net_id); char *hexid; + int ret; if (!test_bit(NFSD4_CLIENT_STABLE, &clp->cl_flags)) return; @@ -1336,9 +1341,13 @@ nfsd4_umh_cltrack_remove(struct nfs4_client *clp) } nfsd4_cltrack_upcall_lock(clp); - if (test_bit(NFSD4_CLIENT_STABLE, &clp->cl_flags) && - nfsd4_umh_cltrack_upcall("remove", hexid, NULL, NULL) == 0) - clear_bit(NFSD4_CLIENT_STABLE, &clp->cl_flags); + if (test_bit(NFSD4_CLIENT_STABLE, &clp->cl_flags)) { + ret = nfsd4_umh_cltrack_upcall("remove", + hexid, NULL, NULL, + nn->umh_token); + if (ret == 0) + clear_bit(NFSD4_CLIENT_STABLE, &clp->cl_flags); + } nfsd4_cltrack_upcall_unlock(clp); kfree(hexid); @@ -1347,8 +1356,9 @@ nfsd4_umh_cltrack_remove(struct nfs4_client *clp)
[RFC PATCH 5 1/7] kmod - add workqueue service thread store
From: Ian Kent Persistent use of process information is needed for contained execution in a namespace other than the root init namespace. Use a simple random token as a key to create and store thread information in a hashed list for use by the usermode helper thread runner. Signed-off-by: Ian Kent Cc: Benjamin Coddington Cc: Al Viro Cc: J. Bruce Fields Cc: David Howells Cc: Trond Myklebust Cc: Oleg Nesterov Cc: Eric W. Biederman Cc: Jeff Layton --- include/linux/kmod.h |3 + kernel/kmod.c| 179 ++ 2 files changed, 181 insertions(+), 1 deletion(-) diff --git a/include/linux/kmod.h b/include/linux/kmod.h index 0555cc6..fa46722 100644 --- a/include/linux/kmod.h +++ b/include/linux/kmod.h @@ -66,6 +66,9 @@ struct subprocess_info { void *data; }; +extern int umh_wq_get_token(int token, const char *service); +extern void umh_wq_put_token(int token); + extern int call_usermodehelper(char *path, char **argv, char **envp, int wait); diff --git a/kernel/kmod.c b/kernel/kmod.c index 2777f40..55d20ce 100644 --- a/kernel/kmod.c +++ b/kernel/kmod.c @@ -40,13 +40,30 @@ #include #include #include +#include +#include +#include #include extern int max_threads; +#define KHELPER"khelper" static struct workqueue_struct *khelper_wq; +#define UMH_WQ_HASH_SHIFT 6 +#define UMH_WQ_HASH_SIZE 1 << UMH_WQ_HASH_SHIFT + +struct umh_wq_entry { + int token; + unsigned int count; + struct workqueue_struct *wq; + struct hlist_node umh_wq_hlist; +}; + +static DEFINE_SPINLOCK(umh_wq_hash_lock); +static struct hlist_head umh_wq_hash[UMH_WQ_HASH_SIZE]; + #define CAP_BSET (void *)1 #define CAP_PI (void *)2 @@ -475,6 +492,165 @@ static void helper_unlock(void) wake_up(&running_helpers_waitq); } +static void umh_wq_hash_init(void) +{ + int i; + + for (i = 0; i < UMH_WQ_HASH_SIZE; i++) + INIT_HLIST_HEAD(&umh_wq_hash[i]); +} + +static struct umh_wq_entry *umh_wq_find_entry(int token) +{ + struct umh_wq_entry *this, *entry; + struct hlist_head *bucket; + unsigned int hash; + + hash = hash_32((unsigned long) token, UMH_WQ_HASH_SHIFT); + bucket = &umh_wq_hash[hash]; + + entry = ERR_PTR(-ENOENT); + if (hlist_empty(bucket)) + goto out; + + hlist_for_each_entry(this, bucket, umh_wq_hlist) { + if (this->token == token) { + entry = this; + break; + } + } +out: + return entry; +} + +static struct workqueue_struct *umh_find_wq(int token, unsigned int nowait) +{ + struct umh_wq_entry *entry; + unsigned long flags; + + if (!token) + return khelper_wq; + + if (nowait) + spin_lock_irqsave(&umh_wq_hash_lock, flags); + else + spin_lock(&umh_wq_hash_lock); + entry = umh_wq_find_entry(token); + if (nowait) + spin_unlock_irqrestore(&umh_wq_hash_lock, flags); + else + spin_unlock(&umh_wq_hash_lock); + + return entry->wq; +} + +/** + * umh_wq_get_token - create service thread and return an identifying token + * @token: token of an existing service thread or 0 to create a new + *service thread. + * @name: service name to be appended to "khelper" for identification. + * + * Returns a token that used with calls to call_usermode_helper_service(). + * If token corresponds to an existing service thread its reference count + * is increased and the token returned. On failure returns a negative errno. + */ +int umh_wq_get_token(int token, const char *service) +{ + struct workqueue_struct *wq; + char *wq_name; + int wq_name_len; + struct umh_wq_entry *entry; + struct hlist_head *bucket; + unsigned int hash; + unsigned int new_token; + + if (token) { + spin_lock(&umh_wq_hash_lock); + entry = umh_wq_find_entry(token); + if (entry) { + entry->count++; + spin_unlock(&umh_wq_hash_lock); + return token; + } + spin_unlock(&umh_wq_hash_lock); + } + + if (!service) + return -EINVAL; + + wq_name_len = sizeof(KHELPER) + strlen(service) + 1; + wq_name = kmalloc(wq_name_len, GFP_KERNEL); + if (!wq_name) + return -ENOMEM; + strcpy(wq_name, "KHELPER-"); + strcat(wq_name, service); + + entry = kzalloc(sizeof(struct umh_wq_entry), GFP_KERNEL); + if (!entry) { + kfree(wq_name); + return -ENOMEM; + } + + wq = create_singlethread_workqueue(wq_name); + if (IS_ERR(wq)) { + kfree(wq_name); +
[RFC PATCH v5 0/7] Another attempt at contained helper execution
Following Eric's comments and in light of the most recent nfs and keys patches here is yet another attempt at the basis of contained usermode helper execution. Initially I thought that creating threads to be used when executing a helper wouldn't be feasible because the number of threads would be far too large but the recent nfs and keys patches make me think that's probably not the case. There's more work to do on this, namely identifying already existing threads for a requested environment, error handling for environments that have gone away due to summary execution and similar. But I'd like to get feedback as to whether I'm on the right track and what I might be missing before spending more time on it. --- Ian Kent (7): kmod - add workqueue service thread store kmod - teach usermodehelper to use service workqueues nfsd - use service thread if not executing in init namespace nfs - cache_lib use service thread if not executing in init namespace nfs - objlayout use service thread if not executing in init namespace KEYS - use correct memory allocation flag in call_usermodehelper_keys() KEYS: exec request key within service thread of key creator fs/nfs/cache_lib.c |7 + fs/nfs/objlayout/objlayout.c | 14 +++ fs/nfsd/netns.h |3 + fs/nfsd/nfs4recover.c| 48 ++--- fs/nfsd/nfsctl.c |6 + include/linux/key.h |3 + include/linux/kmod.h |8 ++ include/linux/sunrpc/cache.h |2 kernel/kmod.c| 217 -- net/sunrpc/cache.c |5 + security/keys/gc.c |2 security/keys/key.c |5 + security/keys/request_key.c | 38 ++- 13 files changed, 323 insertions(+), 35 deletions(-) -- Ian -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[RFC PATCH 5 2/7] kmod - teach usermodehelper to use service workqueues
The call_usermodehelper() function executes all binaries in the global "init" root context. This doesn't allow a binary to be run within a namespace environment such as a container. To do this the namespace environment of the contaner must be available to provide the required execution environment. This can be done by creating a service thread, identified by issuing a token identifier, used when executing the helper with a function that takes the token as a parameter. Signed-off-by: Ian Kent Cc: Benjamin Coddington Cc: Al Viro Cc: J. Bruce Fields Cc: David Howells Cc: Trond Myklebust Cc: Oleg Nesterov Cc: Eric W. Biederman Cc: Jeff Layton --- include/linux/kmod.h |5 + kernel/kmod.c| 38 +++--- 2 files changed, 36 insertions(+), 7 deletions(-) diff --git a/include/linux/kmod.h b/include/linux/kmod.h index fa46722..9a9fcb3 100644 --- a/include/linux/kmod.h +++ b/include/linux/kmod.h @@ -56,6 +56,7 @@ struct file; struct subprocess_info { struct work_struct work; struct completion *complete; + int wq_token; char *path; char **argv; char **envp; @@ -72,6 +73,10 @@ extern void umh_wq_put_token(int token); extern int call_usermodehelper(char *path, char **argv, char **envp, int wait); +extern int +call_usermodehelper_service(char *path, char **argv, + char **envp, int token, int wait); + extern struct subprocess_info * call_usermodehelper_setup(char *path, char **argv, char **envp, gfp_t gfp_mask, int (*init)(struct subprocess_info *info, struct cred *new), diff --git a/kernel/kmod.c b/kernel/kmod.c index 55d20ce..47c5ff5 100644 --- a/kernel/kmod.c +++ b/kernel/kmod.c @@ -712,6 +712,7 @@ EXPORT_SYMBOL(call_usermodehelper_setup); */ int call_usermodehelper_exec(struct subprocess_info *sub_info, int wait) { + static struct workqueue_struct *wq; DECLARE_COMPLETION_ONSTACK(done); int retval = 0; @@ -720,7 +721,8 @@ int call_usermodehelper_exec(struct subprocess_info *sub_info, int wait) return -EINVAL; } helper_lock(); - if (!khelper_wq || usermodehelper_disabled) { + wq = umh_find_wq(sub_info->wq_token, wait); + if (!wq || usermodehelper_disabled) { retval = -EBUSY; goto out; } @@ -732,7 +734,7 @@ int call_usermodehelper_exec(struct subprocess_info *sub_info, int wait) sub_info->complete = (wait == UMH_NO_WAIT) ? NULL : &done; sub_info->wait = wait; - queue_work(khelper_wq, &sub_info->work); + queue_work(wq, &sub_info->work); if (wait == UMH_NO_WAIT)/* task has freed sub_info */ goto unlock; @@ -759,19 +761,21 @@ unlock: EXPORT_SYMBOL(call_usermodehelper_exec); /** - * call_usermodehelper() - prepare and start a usermode application + * call_usermodehelper_service() - start a usermode application within + * a service environment. * @path: path to usermode executable * @argv: arg vector for process * @envp: environment for process + * @token: token corresponding to a service environment obtained by a + *call to umh_wq_get_token(). * @wait: wait for the application to finish and return status. *when UMH_NO_WAIT don't wait at all, but you get no useful error back *when the program couldn't be exec'ed. This makes it safe to call *from interrupt context. - * - * This function is the equivalent to use call_usermodehelper_setup() and - * call_usermodehelper_exec(). */ -int call_usermodehelper(char *path, char **argv, char **envp, int wait) +int call_usermodehelper_service(char *path, + char **argv, char **envp, + int token, int wait) { struct subprocess_info *info; gfp_t gfp_mask = (wait == UMH_NO_WAIT) ? GFP_ATOMIC : GFP_KERNEL; @@ -780,9 +784,29 @@ int call_usermodehelper(char *path, char **argv, char **envp, int wait) NULL, NULL, NULL); if (info == NULL) return -ENOMEM; + info->wq_token = token; return call_usermodehelper_exec(info, wait); } +EXPORT_SYMBOL(call_usermodehelper_service); + +/** + * call_usermodehelper() - prepare and start a usermode application + * @path: path to usermode executable + * @argv: arg vector for process + * @envp: environment for process + * @wait: wait for the application to finish and return status. + *when UMH_NO_WAIT don't wait at all, but you get no useful error back + *when the program couldn't be exec'ed. This makes it safe to call + *from interrupt context. + * + * This function is the equivalent to using call_usermodehelper_setup() and + * call_usermodehelper_exec(). + */ +int call_userm
Re: [RFC PATCH v4 03/12] vfs - move mnt_namespace definition to linux/mount.h
On Thu, 2015-03-19 at 20:14 -0500, Eric W. Biederman wrote: > Ian Kent writes: > > 2> On Thu, 2015-03-19 at 19:47 +, Al Viro wrote: > >> On Tue, Mar 17, 2015 at 10:45:09AM +0800, Ian Kent wrote: > >> > From: Ian Kent > >> > > >> > The mnt_namespace definition will be needed by the usermode helper > >> > contained execution implementation, move it to include/linux/mount.h. > >> > >> I really don't like that. AFAICS, the root of the evil is that fscking > >> nsproxy keeps a pointer to mnt_namespace while all it really needs to > >> know about is the address of ns_common field embedded into it. Let's > >> see... > > > > Thought that might be the case. > > > >> > >> fs/namespace.c:697: struct mnt_namespace *ns = > >> current->nsproxy->mnt_ns; > >> fs/namespace.c:770: return mnt->mnt_ns == current->nsproxy->mnt_ns; > >> fs/namespace.c:1502:return > >> ns_capable(current->nsproxy->mnt_ns->user_ns, CAP_SYS_ADMIN); > >> fs/namespace.c:1587:return current->nsproxy->mnt_ns->seq >= > >> mnt_ns->seq; > >> fs/namespace.c:2293:struct user_namespace *user_ns = > >> current->nsproxy->mnt_ns->user_ns; > >> fs/namespace.c:2961:touch_mnt_namespace(current->nsproxy->mnt_ns); > >> fs/namespace.c:3003:init_task.nsproxy->mnt_ns = ns; > >> fs/namespace.c:3101:ns_root.mnt = ¤t->nsproxy->mnt_ns->root->mnt; > >> fs/namespace.c:3119:struct mnt_namespace *ns = > >> current->nsproxy->mnt_ns; > >> fs/namespace.c:3159:ns = &nsproxy->mnt_ns->ns; > >> fs/namespace.c:3187:put_mnt_ns(nsproxy->mnt_ns); > >> fs/namespace.c:3188:nsproxy->mnt_ns = mnt_ns; > >> fs/pnode.c:282: user_ns = current->nsproxy->mnt_ns->user_ns; > >> fs/proc_namespace.c:246:if (!nsp || !nsp->mnt_ns) { > >> fs/proc_namespace.c:251:ns = nsp->mnt_ns; > >> include/linux/nsproxy.h:33: struct mnt_namespace *mnt_ns; > >> kernel/nsproxy.c:37:.mnt_ns = NULL, > >> kernel/nsproxy.c:70:new_nsp->mnt_ns = copy_mnt_ns(flags, > >> tsk->nsproxy->mnt_ns, user_ns, new_fs); > >> kernel/nsproxy.c:71:if (IS_ERR(new_nsp->mnt_ns)) { > >> kernel/nsproxy.c:72:err = PTR_ERR(new_nsp->mnt_ns); > >> kernel/nsproxy.c:113: if (new_nsp->mnt_ns) > >> kernel/nsproxy.c:114: put_mnt_ns(new_nsp->mnt_ns); > >> kernel/nsproxy.c:160: if (ns->mnt_ns) > >> kernel/nsproxy.c:161: put_mnt_ns(ns->mnt_ns); > >> > >> OK, so we need > >> > >> a) add in fs/mount.h > >> static inline struct mnt_namespace *current_mnt_ns(void) > >> { > >>return current->nsproxy->mnt_ns; > >> } > >> > >> and replace a lot of open-coded instances. > >> > >> b) lift to_mnt_ns() into fs/mount.h (as static inline) > >> > >> c) switch put_mnt_ns() to struct ns_common *, replacing put_mnt_ns(ns) > >> with put_mnt_ns(&ns->ns) and using to_mnt_ns() in the body to recover > >> the original. > >> > >> d) make copy_mnt_ns() take and return struct ns_common * (same treatment > >> as for > >> put_mnt_ns() in (c); replace > >> new_nsp->mnt_ns = copy_mnt_ns(flags, tsk->nsproxy->mnt_ns, > >> user_ns, new_fs); > >> if (IS_ERR(new_nsp->mnt_ns)) { > >> err = PTR_ERR(new_nsp->mnt_ns); > >> goto out_ns; > >> } > >> with > >>ns = copy_mnt_ns(flags, &tsk->nsproxy->mnt_ns->ns, user_ns, new_fs); > >>if (IS_ERR(ns)) { > >>err = PTR_ERR(ns); > >>goto out_ns; > >>} > >>new_nsp->mnt_ns = to_mnt_ns(ns); > >> > >> e) replace struct mnt_namespace *mnt_ns with struct ns_common *__mnt_ns in > >> nsproxy.h, deal with users of mnt_ns by > >> * everywhere in the tree replace put_mnt_ns(&...->mnt_ns->ns) with > >> put_mnt_ns(...->__mnt_ns) (i.e. modify the lines modified by (c) again). > >> * in fs/namespace.c replace > >>init_task.nsproxy->mnt_ns = ns; > >> with > >>init_task.nsproxy->__mnt_ns = &ns->ns; > >> replace > >>ns = &nsproxy->mnt_ns->ns; > >> with > >>ns = ns
Re: [RFC PATCH v4 00/12] Second attempt at contained helper execution
On Thu, 2015-03-19 at 16:38 -0500, Eric W. Biederman wrote: > Ian Kent writes: > > > Here is another update to the attempt at contained helper execution. > > > > The main change is I've tried to incorporate Oleg's suggestions > > of directly constructing the namespaces rather than using the > > open/setns approach and the addition of a namespace hash store. > > > > I'm not particularly happy with this so far as there are a bunch > > of ref counted objects and I've almost certainly got that wrong. > > But also there are object lifetime problems, some I'm aware of > > and for sure others I'm not. Also there is the integrity of the > > thread runner process. I haven't performed a double fork on thread > > execution, it might be painful to implement, so the thread runner > > might end up with the wrong namespace setup if an error occurs. > > > > Anyway, I've decided to stop spinning my wheels with this and > > post an update in the hope that others can offer suggestions to > > help and, of course, point out things I've missed. > > > > The other change has been to the nfs and KEYS patches. > > I've introduced the ability to get a token that can be used to > > save namespace information for later execution and I've attempted > > to use that for persistent namespace execution, as was discussed > > previously. > > > > I'm not at all sure I've done this in a sensible way but the > > token does need to be accessible at helper execution time which > > is why I've done it this way. > > > > I definitely need advice here too. Thanks for offering your advice once again Eric. > > As far as I can tell this patchset continues to be broken for ignoring > my earlier advice. Ignoring is the wrong word. I am ignorant of aspects of the bigger picture here but working on this is helping with that quite a bit and your continued advice is very much needed. > > This patchset provides an escape from cgroup, lsm, rlimit, and > seccomp policy. OK. > > This patchset does not appear particularly nice in how it uses > namespaces. Yeah, I definitely agree with that. > > The only safe and sane way to do this is to have a kernel thread with > all of the proper attributes configured waiting around ready to start > the user mode helper. I originally thought that wouldn't be viable due to the potential number of threads that would be needed. I still think that a thread per mountpoint, as we originally discussed, won't work but looking at nfsd, nfs and the keys code for the recent patch series makes me think that there might not be so many threads needed and that depends on choosing a better place to start the threads. > > The problem you are trying to solve is so hard that we totally failed to > solve it outside of the container case. Which is why we have kthreadd. > I will be very surprised if you can figure out how to cleanly solve the > problem the way you are attacking it. So the previous approach of using file_open_root()/setns_inode() is equally broken in the same respects as you mention above? You didn't mention on that before? I get that the problem is hard to solve but I'd rather not give up on it. Maybe the token I use could relate to a previously created kernel thread instead of namespace information. LOL, then you can describe what I've done wrong creating the kernel threads as well, ;) > > Eric > > -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC PATCH v4 03/12] vfs - move mnt_namespace definition to linux/mount.h
On Thu, 2015-03-19 at 19:47 +, Al Viro wrote: > On Tue, Mar 17, 2015 at 10:45:09AM +0800, Ian Kent wrote: > > From: Ian Kent > > > > The mnt_namespace definition will be needed by the usermode helper > > contained execution implementation, move it to include/linux/mount.h. > > I really don't like that. AFAICS, the root of the evil is that fscking > nsproxy keeps a pointer to mnt_namespace while all it really needs to > know about is the address of ns_common field embedded into it. Let's see... Thought that might be the case. > > fs/namespace.c:697: struct mnt_namespace *ns = current->nsproxy->mnt_ns; > fs/namespace.c:770: return mnt->mnt_ns == current->nsproxy->mnt_ns; > fs/namespace.c:1502:return ns_capable(current->nsproxy->mnt_ns->user_ns, > CAP_SYS_ADMIN); > fs/namespace.c:1587:return current->nsproxy->mnt_ns->seq >= mnt_ns->seq; > fs/namespace.c:2293:struct user_namespace *user_ns = > current->nsproxy->mnt_ns->user_ns; > fs/namespace.c:2961:touch_mnt_namespace(current->nsproxy->mnt_ns); > fs/namespace.c:3003:init_task.nsproxy->mnt_ns = ns; > fs/namespace.c:3101:ns_root.mnt = ¤t->nsproxy->mnt_ns->root->mnt; > fs/namespace.c:3119:struct mnt_namespace *ns = current->nsproxy->mnt_ns; > fs/namespace.c:3159:ns = &nsproxy->mnt_ns->ns; > fs/namespace.c:3187:put_mnt_ns(nsproxy->mnt_ns); > fs/namespace.c:3188:nsproxy->mnt_ns = mnt_ns; > fs/pnode.c:282: user_ns = current->nsproxy->mnt_ns->user_ns; > fs/proc_namespace.c:246:if (!nsp || !nsp->mnt_ns) { > fs/proc_namespace.c:251:ns = nsp->mnt_ns; > include/linux/nsproxy.h:33: struct mnt_namespace *mnt_ns; > kernel/nsproxy.c:37:.mnt_ns = NULL, > kernel/nsproxy.c:70:new_nsp->mnt_ns = copy_mnt_ns(flags, > tsk->nsproxy->mnt_ns, user_ns, new_fs); > kernel/nsproxy.c:71:if (IS_ERR(new_nsp->mnt_ns)) { > kernel/nsproxy.c:72:err = PTR_ERR(new_nsp->mnt_ns); > kernel/nsproxy.c:113: if (new_nsp->mnt_ns) > kernel/nsproxy.c:114: put_mnt_ns(new_nsp->mnt_ns); > kernel/nsproxy.c:160: if (ns->mnt_ns) > kernel/nsproxy.c:161: put_mnt_ns(ns->mnt_ns); > > OK, so we need > > a) add in fs/mount.h > static inline struct mnt_namespace *current_mnt_ns(void) > { > return current->nsproxy->mnt_ns; > } > > and replace a lot of open-coded instances. > > b) lift to_mnt_ns() into fs/mount.h (as static inline) > > c) switch put_mnt_ns() to struct ns_common *, replacing put_mnt_ns(ns) > with put_mnt_ns(&ns->ns) and using to_mnt_ns() in the body to recover > the original. > > d) make copy_mnt_ns() take and return struct ns_common * (same treatment as > for > put_mnt_ns() in (c); replace > new_nsp->mnt_ns = copy_mnt_ns(flags, tsk->nsproxy->mnt_ns, user_ns, > new_fs); > if (IS_ERR(new_nsp->mnt_ns)) { > err = PTR_ERR(new_nsp->mnt_ns); > goto out_ns; > } > with > ns = copy_mnt_ns(flags, &tsk->nsproxy->mnt_ns->ns, user_ns, new_fs); > if (IS_ERR(ns)) { > err = PTR_ERR(ns); > goto out_ns; > } > new_nsp->mnt_ns = to_mnt_ns(ns); > > e) replace struct mnt_namespace *mnt_ns with struct ns_common *__mnt_ns in > nsproxy.h, deal with users of mnt_ns by > * everywhere in the tree replace put_mnt_ns(&...->mnt_ns->ns) with > put_mnt_ns(...->__mnt_ns) (i.e. modify the lines modified by (c) again). > * in fs/namespace.c replace > init_task.nsproxy->mnt_ns = ns; > with > init_task.nsproxy->__mnt_ns = &ns->ns; > replace > ns = &nsproxy->mnt_ns->ns; > with > ns = nsproxy->__mnt_ns; > replace > nsproxy->mnt_ns = mnt_ns; > with > nsproxy>__mnt_ns = &mnt_ns->ns; > * in fs/proc_namespace.c replace > ns = nsp->mnt_ns; > with > ns = to_mnt_ns(nsp->__mnt_ns); > and > if (!nsp || !nsp->mnt_ns) { > with > if (!nsp || !nsp->__mnt_ns) { > * in kernel/nsproxy.c: replace > ns = copy_mnt_ns(flags, &tsk->nsproxy->mnt_ns->ns, user_ns, new_fs); > added in (d) with > ns = copy_mnt_ns(flags, tsk->nsproxy->__mnt_ns, user_ns, new_fs); > and > new_nsp->mnt_ns = to_mnt_ns(ns); > with > new_nsp->__mnt_ns = ns; > The rest of mnt_ns in that file get replaced by __mnt_ns. > * in current_mnt_ns() replace ...->mnt_ns with to_mnt_ns(...->__mnt_ns) > > Do you want me to push such a series in vfs.git? After such 5 commits we have > no pointers to struct mnt_namespace in nsproxy.h *and* no need for your > patches > to dig into ->mnt_ns->ns - we already have that as ->__mnt_ns. Yes please, I'd be more confident if you did this than me, there's already enough to worry about with the series. Ian -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[RFC PATCH v4 02/12] kmod - rename call_usermodehelper() flags parameter
From: Ian Kent The wait parameter of call_usermodehelper() is not quite a parameter that describes the wait behaviour alone and will later be used to request execution within the current namespaces. This flag is tied to the wait field of the subprocess_info structure which is also a field that doesn't specify wait behaviour alone and is used to hold the passed flags information. So change both the parameter and structure field name to flags. Signed-off-by: Ian Kent Cc: Benjamin Coddington Cc: Al Viro Cc: J. Bruce Fields Cc: David Howells Cc: Trond Myklebust Cc: Oleg Nesterov Cc: Eric W. Biederman Cc: Jeff Layton --- include/linux/kmod.h |6 +++--- kernel/kmod.c| 32 +--- 2 files changed, 20 insertions(+), 18 deletions(-) diff --git a/include/linux/kmod.h b/include/linux/kmod.h index 0555cc6..e647ddb 100644 --- a/include/linux/kmod.h +++ b/include/linux/kmod.h @@ -59,7 +59,7 @@ struct subprocess_info { char *path; char **argv; char **envp; - int wait; + unsigned int flags; int retval; int (*init)(struct subprocess_info *info, struct cred *new); void (*cleanup)(struct subprocess_info *info); @@ -67,7 +67,7 @@ struct subprocess_info { }; extern int -call_usermodehelper(char *path, char **argv, char **envp, int wait); +call_usermodehelper(char *path, char **argv, char **envp, unsigned int flags); extern struct subprocess_info * call_usermodehelper_setup(char *path, char **argv, char **envp, gfp_t gfp_mask, @@ -75,7 +75,7 @@ call_usermodehelper_setup(char *path, char **argv, char **envp, gfp_t gfp_mask, void (*cleanup)(struct subprocess_info *), void *data); extern int -call_usermodehelper_exec(struct subprocess_info *info, int wait); +call_usermodehelper_exec(struct subprocess_info *info, unsigned int flags); extern struct ctl_table usermodehelper_table[]; diff --git a/kernel/kmod.c b/kernel/kmod.c index 2777f40..e968e2d 100644 --- a/kernel/kmod.c +++ b/kernel/kmod.c @@ -259,7 +259,7 @@ static int call_usermodehelper(void *data) out: sub_info->retval = retval; /* wait_for_helper() will call umh_complete if UHM_WAIT_PROC. */ - if (!(sub_info->wait & UMH_WAIT_PROC)) + if (!(sub_info->flags & UMH_WAIT_PROC)) umh_complete(sub_info); if (!retval) return 0; @@ -310,7 +310,7 @@ static void __call_usermodehelper(struct work_struct *work) container_of(work, struct subprocess_info, work); pid_t pid; - if (sub_info->wait & UMH_WAIT_PROC) + if (sub_info->flags & UMH_WAIT_PROC) pid = kernel_thread(wait_for_helper, sub_info, CLONE_FS | CLONE_FILES | SIGCHLD); else @@ -525,16 +525,17 @@ EXPORT_SYMBOL(call_usermodehelper_setup); /** * call_usermodehelper_exec - start a usermode application * @sub_info: information about the subprocessa - * @wait: wait for the application to finish and return status. - *when UMH_NO_WAIT don't wait at all, but you get no useful error back - *when the program couldn't be exec'ed. This makes it safe to call - *from interrupt context. + * @flags: flag to indicate whether to wait for the application to finish + * and return status. If UMH_NO_WAIT is set don't wait at all, but + * you get no useful error back when the program couldn't be exec'ed. + * This makes it safe to call from interrupt context. * * Runs a user-space application. The application is started * asynchronously if wait is not set, and runs as a child of keventd. * (ie. it runs with full root capabilities). */ -int call_usermodehelper_exec(struct subprocess_info *sub_info, int wait) +int call_usermodehelper_exec(struct subprocess_info *sub_info, +unsigned int flags) { DECLARE_COMPLETION_ONSTACK(done); int retval = 0; @@ -553,14 +554,14 @@ int call_usermodehelper_exec(struct subprocess_info *sub_info, int wait) * This makes it possible to use umh_complete to free * the data structure in case of UMH_NO_WAIT. */ - sub_info->complete = (wait == UMH_NO_WAIT) ? NULL : &done; - sub_info->wait = wait; + sub_info->complete = (flags & UMH_NO_WAIT) ? NULL : &done; + sub_info->flags = flags; queue_work(khelper_wq, &sub_info->work); - if (wait == UMH_NO_WAIT)/* task has freed sub_info */ + if (flags & UMH_NO_WAIT)/* task has freed sub_info */ goto unlock; - if (wait & UMH_KILLABLE) { + if (flags & UMH_KILLABLE) { retval = wait_for_completion_killable(&done); if (!retval) goto wait_done; @@ -587,7 +588,7 @@ EXPORT_SYMBOL(call_usermode
[RFC PATCH v4 05/12] kmod - teach call_usermodehelper() to use a namespace
From: Ian Kent The call_usermodehelper() function executes all binaries in the global "init" root context. This doesn't allow a binary to be run within a namespace (eg. the namespaces of a container). The init process of the callers environment is used to setup the namespaces in almost the same way the root init process when the UMH_USE_NS flag is used. Signed-off-by: Ian Kent Cc: Benjamin Coddington Cc: Al Viro Cc: J. Bruce Fields Cc: David Howells Cc: Trond Myklebust Cc: Oleg Nesterov Cc: Eric W. Biederman Cc: Jeff Layton --- kernel/kmod.c | 66 + 1 file changed, 66 insertions(+) diff --git a/kernel/kmod.c b/kernel/kmod.c index 213dbe0..d6ee21a 100644 --- a/kernel/kmod.c +++ b/kernel/kmod.c @@ -56,6 +56,8 @@ static kernel_cap_t usermodehelper_inheritable = CAP_FULL_SET; static DEFINE_SPINLOCK(umh_sysctl_lock); static DECLARE_RWSEM(umhelper_sem); +static void umh_put_nsproxy(struct subprocess_info *); + #ifdef CONFIG_MODULES /* @@ -194,6 +196,7 @@ static void call_usermodehelper_freeinfo(struct subprocess_info *info) { if (info->cleanup) (*info->cleanup)(info); + umh_put_nsproxy(info); kfree(info); } @@ -565,6 +568,61 @@ static void helper_unlock(void) wake_up(&running_helpers_waitq); } +#ifndef CONFIG_NAMESPACES +static int umh_get_nsproxy(struct subprocess_info *sub_info) +{ + return -ENOTSUP; +} + +static void umh_put_nsproxy(struct subprocess_info *sub_info) +{ +} +#else +static int umh_get_nsproxy(struct subprocess_info *sub_info) +{ + struct umh_ns_info *nsinfo = &sub_info->nsinfo; + struct task_struct *tsk; + struct user_namespace *user_ns; + struct nsproxy *new; + int err = 0; + + rcu_read_lock(); + tsk = find_task_by_vpid(1); + if (tsk) + get_task_struct(tsk); + rcu_read_unlock(); + if (!tsk) { + err = -ESRCH; + goto out; + } + + user_ns = get_user_ns(tsk->cred->user_ns); + + new = create_new_namespaces(0, tsk, user_ns, tsk->fs); + if (IS_ERR(new)) { + err = PTR_ERR(new); + put_user_ns(user_ns); + put_task_struct(tsk); + goto out; + } + + put_task_struct(tsk); + + nsinfo->nsproxy = new; + nsinfo->user_ns = user_ns; +out: + return err; +} + +static void umh_put_nsproxy(struct subprocess_info *sub_info) +{ + if (sub_info->nsinfo.nsproxy) { + put_nsproxy(sub_info->nsinfo.nsproxy); + put_user_ns(sub_info->nsinfo.user_ns); + } +} +#endif + /** * call_usermodehelper_setup - prepare to call a usermode helper * @path: path to usermode executable @@ -697,6 +755,14 @@ int call_usermodehelper(char *path, if (info == NULL) return -ENOMEM; + if (flags & UMH_USE_NS) { + int err = umh_get_nsproxy(info); + if (err) { + kfree(info); + return err; + } + } + return call_usermodehelper_exec(info, flags); } EXPORT_SYMBOL(call_usermodehelper); -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[RFC PATCH v4 04/12] kmod - add namespace aware thread runner
From: Ian Kent Make usermode helper thread runner namespace aware. Signed-off-by: Ian Kent Cc: Benjamin Coddington Cc: Al Viro Cc: J. Bruce Fields Cc: David Howells Cc: Trond Myklebust Cc: Oleg Nesterov Cc: Eric W. Biederman Cc: Jeff Layton --- include/linux/kmod.h | 12 ++ kernel/kmod.c| 98 -- 2 files changed, 106 insertions(+), 4 deletions(-) diff --git a/include/linux/kmod.h b/include/linux/kmod.h index e647ddb..64c81c9 100644 --- a/include/linux/kmod.h +++ b/include/linux/kmod.h @@ -25,6 +25,7 @@ #include #include #include +#include #define KMOD_PATH_LEN 256 @@ -52,6 +53,14 @@ struct file; #define UMH_WAIT_EXEC 1 /* wait for the exec, but not the process */ #define UMH_WAIT_PROC 2 /* wait for the process to complete */ #define UMH_KILLABLE 4 /* wait for EXEC/PROC killable */ +#define UMH_USE_NS 32 /* exec using caller's init namespace */ + +#ifdef CONFIG_NAMESPACES +struct umh_ns_info { + struct nsproxy *nsproxy; + struct user_namespace *user_ns; +}; +#endif struct subprocess_info { struct work_struct work; @@ -64,6 +73,9 @@ struct subprocess_info { int (*init)(struct subprocess_info *info, struct cred *new); void (*cleanup)(struct subprocess_info *info); void *data; +#ifdef CONFIG_NAMESPACES + struct umh_ns_info nsinfo; +#endif }; extern int diff --git a/kernel/kmod.c b/kernel/kmod.c index e968e2d..213dbe0 100644 --- a/kernel/kmod.c +++ b/kernel/kmod.c @@ -39,6 +39,7 @@ #include #include #include +#include #include #include @@ -303,11 +304,8 @@ static int wait_for_helper(void *data) do_exit(0); } -/* This is run by khelper thread */ -static void __call_usermodehelper(struct work_struct *work) +static pid_t umh_kernel_thread(struct subprocess_info *sub_info) { - struct subprocess_info *sub_info = - container_of(work, struct subprocess_info, work); pid_t pid; if (sub_info->flags & UMH_WAIT_PROC) @@ -316,7 +314,99 @@ static void __call_usermodehelper(struct work_struct *work) else pid = kernel_thread(call_usermodehelper, sub_info, SIGCHLD); + return pid; +} + +#ifndef CONFIG_NAMESPACES +static pid_t umh_kernel_thread_in_ns(struct subprocess_info *sub_info) +{ + return -ENOTSUP; +} +#else +static pid_t umh_kernel_thread_in_ns(struct subprocess_info *sub_info) +{ + struct umh_ns_info *info = &sub_info->nsinfo; + struct nsproxy *saved_nsp, *new_nsp; + struct user_namespace *user_ns; + struct pid_namespace *pid_ns; + struct mnt_namespace *mnt_ns; + pid_t pid; + int err; + + saved_nsp = current->nsproxy; + get_nsproxy(saved_nsp); + + new_nsp = info->nsproxy; + get_nsproxy(new_nsp); + + user_ns = get_user_ns(current->cred->user_ns); + if (info->user_ns) { + err = user_ns->ns.ops->install(new_nsp, &info->user_ns->ns); + if (err) + goto out; + } + + /* May need to wait4() completion so a pid valid in the +* thread runners namespace is needed. Install current +* pid ns in the nsproxy. +*/ + pid_ns = current->nsproxy->pid_ns_for_children; + if (pid_ns) { + err = pid_ns->ns.ops->install(new_nsp, &pid_ns->ns); + if (err) + goto out_user_ns; + } + + /* The mount namespace install function is a little +* more than a no-op, as the install functions of the +* other namespace types are in our case, we need to +* call it to setup current fs root and pwd. +*/ + mnt_ns = current->nsproxy->mnt_ns; + err = mnt_ns->ns.ops->install(new_nsp, &new_nsp->mnt_ns->ns); + if (err) + goto out_user_ns; + /* Finally, switch to the nsproxy of the init namespace +* of the caller. +*/ + switch_task_namespaces(current, new_nsp); + + pid = umh_kernel_thread(sub_info); + + mnt_ns->ns.ops->install(saved_nsp, &saved_nsp->mnt_ns->ns); + + if (info->user_ns) + user_ns->ns.ops->install(saved_nsp, &user_ns->ns); + + switch_task_namespaces(current, saved_nsp); + + put_user_ns(user_ns); + + return pid; + +out_user_ns: + if (info->user_ns) + user_ns->ns.ops->install(saved_nsp, &user_ns->ns); +out: + put_user_ns(user_ns); + put_nsproxy(new_nsp); + put_nsproxy(saved_nsp); + return err; +} +#endif + +/* This is run by khelper thread */ +static void __call_usermodehelper(struct work_struct *work) +{ + struct subprocess_info *sub_info = + container_of(work,
[RFC PATCH v4 09/12] nfs - cache_lib use namespace if not executing in init namespace
From: Ian Kent If pipefs is registered within a namespace other than the root init namespace subsequent pipefs requests should be run within the init namespace of registration. Signed-off-by: Ian Kent Cc: Benjamin Coddington Cc: Al Viro Cc: J. Bruce Fields Cc: David Howells Cc: Trond Myklebust Cc: Oleg Nesterov Cc: Eric W. Biederman Cc: Jeff Layton --- fs/nfs/cache_lib.c |7 ++- include/linux/sunrpc/cache.h |2 ++ net/sunrpc/cache.c |5 + 3 files changed, 13 insertions(+), 1 deletion(-) diff --git a/fs/nfs/cache_lib.c b/fs/nfs/cache_lib.c index 5f7b053..4f381ad 100644 --- a/fs/nfs/cache_lib.c +++ b/fs/nfs/cache_lib.c @@ -48,7 +48,12 @@ int nfs_cache_upcall(struct cache_detail *cd, char *entry_name) if (nfs_cache_getent_prog[0] == '\0') goto out; - ret = call_usermodehelper(argv[0], argv, envp, UMH_WAIT_EXEC); + if (cd->u.pipefs.umh_token) { + long token = cd->u.pipefs.umh_token; + ret = call_usermodehelper_ns(argv[0], argv, envp, +UMH_WAIT_EXEC, token); + } else + ret = call_usermodehelper(argv[0], argv, envp, UMH_WAIT_EXEC); /* * Disable the upcall mechanism if we're getting an ENOENT or * EACCES error. The admin can re-enable it on the fly by using diff --git a/include/linux/sunrpc/cache.h b/include/linux/sunrpc/cache.h index 437ddb6..f6c1eb2 100644 --- a/include/linux/sunrpc/cache.h +++ b/include/linux/sunrpc/cache.h @@ -68,6 +68,8 @@ struct cache_detail_procfs { struct cache_detail_pipefs { struct dentry *dir; + /* Namespace token */ + long umh_token; }; struct cache_detail { diff --git a/net/sunrpc/cache.c b/net/sunrpc/cache.c index 5199bb1..a635efb 100644 --- a/net/sunrpc/cache.c +++ b/net/sunrpc/cache.c @@ -1811,6 +1811,9 @@ int sunrpc_cache_register_pipefs(struct dentry *parent, if (IS_ERR(dir)) return PTR_ERR(dir); cd->u.pipefs.dir = dir; + if (cd->net != &init_net) + cd->u.pipefs.umh_token = +umh_ns_get_token(cd->u.pipefs.umh_token); return 0; } EXPORT_SYMBOL_GPL(sunrpc_cache_register_pipefs); @@ -1819,6 +1822,8 @@ void sunrpc_cache_unregister_pipefs(struct cache_detail *cd) { rpc_remove_cache_dir(cd->u.pipefs.dir); cd->u.pipefs.dir = NULL; + umh_ns_put_token(cd->u.pipefs.umh_token); + cd->u.pipefs.umh_token = 0; } EXPORT_SYMBOL_GPL(sunrpc_cache_unregister_pipefs); -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[RFC PATCH v4 10/12] nfs - objlayout use namespace if not executing in init namespace
From: Ian Kent If the caller is running within a container then execute the usermode helper callback within the init namespace of the container. Signed-off-by: Ian Kent Cc: Benjamin Coddington Cc: Al Viro Cc: J. Bruce Fields Cc: David Howells Cc: Trond Myklebust Cc: Oleg Nesterov Cc: Eric W. Biederman Cc: Jeff Layton --- fs/nfs/objlayout/objlayout.c |7 ++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/fs/nfs/objlayout/objlayout.c b/fs/nfs/objlayout/objlayout.c index 919efd4..00c9a34 100644 --- a/fs/nfs/objlayout/objlayout.c +++ b/fs/nfs/objlayout/objlayout.c @@ -599,9 +599,14 @@ static int __objlayout_upcall(struct __auto_login *login) "PATH=/sbin:/usr/sbin:/bin:/usr/bin", NULL }; + unsigned int umh_flags = UMH_WAIT_PROC; char *argv[8]; int ret; + /* If running within a container use the container namespace */ + if (current->nsproxy->net_ns != &init_net) + umh_flags |= UMH_USE_NS; + if (unlikely(!osd_login_prog[0])) { dprintk("%s: osd_login_prog is disabled\n", __func__); return -EACCES; @@ -620,7 +625,7 @@ static int __objlayout_upcall(struct __auto_login *login) argv[6] = login->systemid_hex; argv[7] = NULL; - ret = call_usermodehelper(argv[0], argv, envp, UMH_WAIT_PROC); + ret = call_usermodehelper(argv[0], argv, envp, umh_flags); /* * Disable the upcall mechanism if we're getting an ENOENT or * EACCES error. The admin can re-enable it on the fly by using -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[RFC PATCH v4 12/12] KEYS: exec request-key within the requesting task's init namespace
From: Ian Kent Containerized request key helper callbacks need the ability to execute a binary in a container's context. To do this calling an in kernel equivalent of setns(2) should be sufficient since the user mode helper execution kernel thread ultimately calls do_execve(). Signed-off-by: Ian Kent Cc: Benjamin Coddington Cc: Al Viro Cc: J. Bruce Fields Cc: David Howells Cc: Trond Myklebust Cc: Oleg Nesterov Cc: Eric W. Biederman Cc: Jeff Layton --- include/linux/key.h |3 +++ security/keys/gc.c |2 ++ security/keys/key.c |4 security/keys/request_key.c | 35 +-- 4 files changed, 42 insertions(+), 2 deletions(-) diff --git a/include/linux/key.h b/include/linux/key.h index e1d4715..89dc2d7 100644 --- a/include/linux/key.h +++ b/include/linux/key.h @@ -209,6 +209,9 @@ struct key { } payload; struct assoc_array keys; }; + + /* Namespace token */ + long umh_token; }; extern struct key *key_alloc(struct key_type *type, diff --git a/security/keys/gc.c b/security/keys/gc.c index c795237..57a0730 100644 --- a/security/keys/gc.c +++ b/security/keys/gc.c @@ -156,6 +156,8 @@ static noinline void key_gc_unused_keys(struct list_head *keys) kfree(key->description); + umh_ns_put_token(key->umh_token); + #ifdef KEY_DEBUGGING key->magic = KEY_DEBUG_MAGIC_X; #endif diff --git a/security/keys/key.c b/security/keys/key.c index aee2ec5..e7ab89d 100644 --- a/security/keys/key.c +++ b/security/keys/key.c @@ -18,6 +18,7 @@ #include #include #include +#include #include "internal.h" struct kmem_cache *key_jar; @@ -309,6 +310,9 @@ struct key *key_alloc(struct key_type *type, const char *desc, /* publish the key by giving it a serial number */ atomic_inc(&user->nkeys); key_alloc_serial(key); + /* If running within a container use the container namespace */ + if (current->nsproxy->net_ns != &init_net) + key->umh_token = umh_ns_get_token(0); error: return key; diff --git a/security/keys/request_key.c b/security/keys/request_key.c index e865f9f..16ac3b0 100644 --- a/security/keys/request_key.c +++ b/security/keys/request_key.c @@ -90,6 +90,31 @@ static int call_usermodehelper_keys(char *path, char **argv, char **envp, } /* + * Call a usermode helper with a specific session keyring and execute + * within a namespace. + */ +static int call_usermodehelper_keys_ns(char *path, char **argv, char **envp, + struct key *session_keyring, + unsigned int wait, long token) +{ + struct subprocess_info *info; + unsigned int gfp_mask = (wait & UMH_NO_WAIT) ? + GFP_ATOMIC : GFP_KERNEL; + + if (token <= 0) + return -EINVAL; + + info = call_usermodehelper_setup_ns(path, argv, envp, gfp_mask, + umh_keys_init, umh_keys_cleanup, + session_keyring, token); + if (!info) + return -ENOMEM; + + key_get(session_keyring); + return call_usermodehelper_exec(info, wait|UMH_USE_NS); +} + +/* * Request userspace finish the construction of a key * - execute "/sbin/request-key " */ @@ -104,6 +129,7 @@ static int call_sbin_request_key(struct key_construction *cons, char *argv[9], *envp[3], uid_str[12], gid_str[12]; char key_str[12], keyring_str[3][12]; char desc[20]; + unsigned int wait = UMH_WAIT_PROC; int ret, i; kenter("{%d},{%d},%s", key->serial, authkey->serial, op); @@ -174,8 +200,13 @@ static int call_sbin_request_key(struct key_construction *cons, argv[i] = NULL; /* do it */ - ret = call_usermodehelper_keys(argv[0], argv, envp, keyring, - UMH_WAIT_PROC); + /* If running within a container use the container namespace */ + if (key->umh_token) + ret = call_usermodehelper_keys_ns(argv[0], argv, envp, + keyring, wait, key->umh_token); + else + ret = call_usermodehelper_keys(argv[0], + argv, envp, keyring, wait); kdebug("usermode -> 0x%x", ret); if (ret >= 0) { /* ret is the exit/wait code */ -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[RFC PATCH v4 07/12] kmod - add call_usermodehelper_ns()
From: Ian Kent Add function call_usermodehelper_ns() to allow passing a namespace token to lookup previously stored namespace information for usermode helper execution. Signed-off-by: Ian Kent Cc: Benjamin Coddington Cc: Al Viro Cc: J. Bruce Fields Cc: David Howells Cc: Trond Myklebust Cc: Oleg Nesterov Cc: Eric W. Biederman Cc: Jeff Layton --- include/linux/kmod.h | 24 + kernel/kmod.c| 96 ++ 2 files changed, 120 insertions(+) diff --git a/include/linux/kmod.h b/include/linux/kmod.h index 77f41ce..a761650 100644 --- a/include/linux/kmod.h +++ b/include/linux/kmod.h @@ -87,9 +87,33 @@ static inline long umh_ns_get_token(long token) static inline void umh_ns_put_token(long token) { } + +static inline int +call_usermodehelper_ns(char *path, char **argv, char **envp, + unsigned int flags, long token) +{ + return -ENOTSUP; +} + +static inline struct subprocess_info * +call_usermodehelper_setup_ns(char *path, char **argv, char **envp, gfp_t gfp_mask, +int (*init)(struct subprocess_info *info, struct cred *new), +void (*cleanup)(struct subprocess_info *), void *data, +long token) +{ + return -ENOTSUP; +} #else extern long umh_ns_get_token(long token); extern void umh_ns_put_token(long token); +extern int +call_usermodehelper_ns(char *path, char **argv, char **envp, + unsigned int flags, long token); +extern struct subprocess_info * +call_usermodehelper_setup_ns(char *path, char **argv, char **envp, gfp_t gfp_mask, +int (*init)(struct subprocess_info *info, struct cred *new), +void (*cleanup)(struct subprocess_info *), void *data, +long token); #endif extern int diff --git a/kernel/kmod.c b/kernel/kmod.c index ddd41f1..d711240 100644 --- a/kernel/kmod.c +++ b/kernel/kmod.c @@ -842,6 +842,62 @@ struct subprocess_info *call_usermodehelper_setup(char *path, char **argv, } EXPORT_SYMBOL(call_usermodehelper_setup); +#ifdef CONFIG_NAMESPACES +/** + * call_usermodehelper_setup_ns - prepare to call a usermode helper + * within a namespace + * @path: path to usermode executable + * @argv: arg vector for process + * @envp: environment for process + * @gfp_mask: gfp mask for memory allocation + * @cleanup: a cleanup function + * @init: an init function + * @data: arbitrary context sensitive data + * @token: token used to locate namespace setup. + * + * Returns either an errno error cast, or a subprocess_info structure. + * This should be passed to call_usermodehelper_exec to exec the process + * and free the structure. + * + * The init function is used to customize the helper process prior to + * exec. A non-zero return code causes the process to error out, exit, + * and return the failure to the calling process + * + * The cleanup function is run just before the subprocess_info is about + * to be freed. This can be used for freeing the argv and envp. The + * Function must be runnable in either a process context or the + * context in which call_usermodehelper_exec is called. + */ +struct subprocess_info *call_usermodehelper_setup_ns(char *path, char **argv, + char **envp, gfp_t gfp_mask, + int (*init)(struct subprocess_info *info, struct cred *new), + void (*cleanup)(struct subprocess_info *info), + void *data, long token) +{ + struct subprocess_info *info; + unsigned int nowait = gfp_mask == GFP_ATOMIC ? 1 : 0; + struct umh_ns_entry *entry; + + info = call_usermodehelper_setup(path, argv, envp, +gfp_mask, NULL, NULL, NULL); + if (!info) + return ERR_PTR(-ENOMEM); + + entry = umh_ns_find_entry(token, nowait); + if (IS_ERR(entry)) { + kfree(info); + info = ERR_CAST(entry); + goto out; + } + get_nsproxy(entry->nsinfo.nsproxy); + info->nsinfo.nsproxy = entry->nsinfo.nsproxy; + info->nsinfo.user_ns = get_user_ns(entry->nsinfo.user_ns); +out: + return info; +} +EXPORT_SYMBOL(call_usermodehelper_setup_ns); +#endif /* CONFIG_NAMESPACES */ + /** * call_usermodehelper_exec - start a usermode application * @sub_info: information about the subprocessa @@ -939,6 +995,46 @@ int call_usermodehelper(char *path, } EXPORT_SYMBOL(call_usermodehelper); +#ifdef CONFIG_NAMESPACES +/** + * call_usermodehelper_ns() - prepare and start a usermode application and + * execute using the stored namspace information + * corresponding to the passed token + * @path: path to usermode executable + * @argv: arg vector for process + * @envp: environment for process + * @flags: wait for the application to finish an
[RFC PATCH v4 08/12] nfsd - use namespace if not executing in init namespace
From: Ian Kent If nfsd is running within a container the client tracking operations should run within the originating container also. Signed-off-by: Ian Kent Cc: Benjamin Coddington Cc: Al Viro Cc: J. Bruce Fields Cc: David Howells Cc: Trond Myklebust Cc: Oleg Nesterov Cc: Eric W. Biederman Cc: Jeff Layton --- fs/nfsd/netns.h |3 +++ fs/nfsd/nfs4recover.c | 48 +++- fs/nfsd/nfsctl.c |6 ++ 3 files changed, 40 insertions(+), 17 deletions(-) diff --git a/fs/nfsd/netns.h b/fs/nfsd/netns.h index ea6749a..c85c13a 100644 --- a/fs/nfsd/netns.h +++ b/fs/nfsd/netns.h @@ -112,6 +112,9 @@ struct nfsd_net { u32 clientid_counter; struct svc_serv *nfsd_serv; + + /* Namespace token */ + long umh_token; }; /* Simple check to find out if a given net was properly initialized */ diff --git a/fs/nfsd/nfs4recover.c b/fs/nfsd/nfs4recover.c index 1c307f0..df13b54 100644 --- a/fs/nfsd/nfs4recover.c +++ b/fs/nfsd/nfs4recover.c @@ -1184,7 +1184,8 @@ nfsd4_cltrack_grace_start(time_t grace_start) } static int -nfsd4_umh_cltrack_upcall(char *cmd, char *arg, char *env0, char *env1) +nfsd4_umh_cltrack_upcall(char *cmd, char *arg, +char *env0, char *env1, long token) { char *envp[3]; char *argv[4]; @@ -1209,7 +1210,11 @@ nfsd4_umh_cltrack_upcall(char *cmd, char *arg, char *env0, char *env1) argv[2] = arg; argv[3] = NULL; - ret = call_usermodehelper(argv[0], argv, envp, UMH_WAIT_PROC); + if (token > 0) + ret = call_usermodehelper_ns(argv[0], argv, envp, +UMH_WAIT_PROC, token); + else + ret = call_usermodehelper(argv[0], argv, envp, UMH_WAIT_PROC); /* * Disable the upcall mechanism if we're getting an ENOENT or EACCES * error. The admin can re-enable it on the fly by using sysfs @@ -1252,14 +1257,8 @@ nfsd4_umh_cltrack_init(struct net *net) struct nfsd_net *nn = net_generic(net, nfsd_net_id); char *grace_start = nfsd4_cltrack_grace_start(nn->boot_time); - /* XXX: The usermode helper s not working in container yet. */ - if (net != &init_net) { - WARN(1, KERN_ERR "NFSD: attempt to initialize umh client " - "tracking in a container!\n"); - return -EINVAL; - } - - ret = nfsd4_umh_cltrack_upcall("init", NULL, grace_start, NULL); + ret = nfsd4_umh_cltrack_upcall("init", NULL, + grace_start, NULL, nn->umh_token); kfree(grace_start); return ret; } @@ -1285,6 +1284,7 @@ nfsd4_umh_cltrack_create(struct nfs4_client *clp) { char *hexid, *has_session, *grace_start; struct nfsd_net *nn = net_generic(clp->net, nfsd_net_id); + int ret; /* * With v4.0 clients, there's little difference in outcome between a @@ -1312,7 +1312,10 @@ nfsd4_umh_cltrack_create(struct nfs4_client *clp) grace_start = nfsd4_cltrack_grace_start(nn->boot_time); nfsd4_cltrack_upcall_lock(clp); - if (!nfsd4_umh_cltrack_upcall("create", hexid, has_session, grace_start)) + ret = nfsd4_umh_cltrack_upcall("create", + hexid, has_session, grace_start, + nn->umh_token); + if (!ret) set_bit(NFSD4_CLIENT_STABLE, &clp->cl_flags); nfsd4_cltrack_upcall_unlock(clp); @@ -1324,7 +1327,9 @@ nfsd4_umh_cltrack_create(struct nfs4_client *clp) static void nfsd4_umh_cltrack_remove(struct nfs4_client *clp) { + struct nfsd_net *nn = net_generic(clp->net, nfsd_net_id); char *hexid; + int ret; if (!test_bit(NFSD4_CLIENT_STABLE, &clp->cl_flags)) return; @@ -1336,9 +1341,13 @@ nfsd4_umh_cltrack_remove(struct nfs4_client *clp) } nfsd4_cltrack_upcall_lock(clp); - if (test_bit(NFSD4_CLIENT_STABLE, &clp->cl_flags) && - nfsd4_umh_cltrack_upcall("remove", hexid, NULL, NULL) == 0) - clear_bit(NFSD4_CLIENT_STABLE, &clp->cl_flags); + if (test_bit(NFSD4_CLIENT_STABLE, &clp->cl_flags)) { + ret = nfsd4_umh_cltrack_upcall("remove", + hexid, NULL, NULL, + nn->umh_token); + if (ret == 0) + clear_bit(NFSD4_CLIENT_STABLE, &clp->cl_flags); + } nfsd4_cltrack_upcall_unlock(clp); kfree(hexid); @@ -1347,8 +1356,9 @@ nfsd4_umh_cltrack_remove(struct nfs4_client *clp) static int nfsd4_umh_cltrack_check(struct nfs4_client *clp) { - int ret; + struct nfsd_net *nn = net_generic(cl
[RFC PATCH v4 11/12] KEYS - use correct memory allocation flag in call_usermodehelper_keys()
From: Ian Kent When call_usermodehelper_keys() is called it assumes it won't be called with the flag UMH_NO_WAIT. Currently that's always the case. Change this to check the flag and use the correct kernel memory allocation flag to guard against future changes. Signed-off-by: Ian Kent Cc: Benjamin Coddington Cc: Al Viro Cc: J. Bruce Fields Cc: David Howells Cc: Trond Myklebust Cc: Oleg Nesterov Cc: Eric W. Biederman Cc: Jeff Layton --- security/keys/request_key.c |4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/security/keys/request_key.c b/security/keys/request_key.c index 486ef6f..e865f9f 100644 --- a/security/keys/request_key.c +++ b/security/keys/request_key.c @@ -76,8 +76,10 @@ static int call_usermodehelper_keys(char *path, char **argv, char **envp, struct key *session_keyring, int wait) { struct subprocess_info *info; + unsigned int gfp_mask = (wait & UMH_NO_WAIT) ? + GFP_ATOMIC : GFP_KERNEL; - info = call_usermodehelper_setup(path, argv, envp, GFP_KERNEL, + info = call_usermodehelper_setup(path, argv, envp, gfp_mask, umh_keys_init, umh_keys_cleanup, session_keyring); if (!info) -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[RFC PATCH v4 06/12] kmod - add namespace info store
From: Ian Kent Persistent use of namespace information is needed where contained execution is needed in a namespace other than the current namespace. Use a simple random token as a key to store namespace information in a hashed list for later usermode helper execution. Signed-off-by: Ian Kent Cc: Benjamin Coddington Cc: Al Viro Cc: J. Bruce Fields Cc: David Howells Cc: Trond Myklebust Cc: Oleg Nesterov Cc: Eric W. Biederman Cc: Jeff Layton --- include/linux/kmod.h | 14 kernel/kmod.c| 185 -- 2 files changed, 193 insertions(+), 6 deletions(-) diff --git a/include/linux/kmod.h b/include/linux/kmod.h index 64c81c9..77f41ce 100644 --- a/include/linux/kmod.h +++ b/include/linux/kmod.h @@ -78,6 +78,20 @@ struct subprocess_info { #endif }; +#ifndef CONFIG_NAMESPACES +static inline long umh_ns_get_token(long token) +{ + return -ENOTSUP; +} + +static inline void umh_ns_put_token(long token) +{ +} +#else +extern long umh_ns_get_token(long token); +extern void umh_ns_put_token(long token); +#endif + extern int call_usermodehelper(char *path, char **argv, char **envp, unsigned int flags); diff --git a/kernel/kmod.c b/kernel/kmod.c index d6ee21a..ddd41f1 100644 --- a/kernel/kmod.c +++ b/kernel/kmod.c @@ -41,6 +41,9 @@ #include #include #include +#include +#include +#include #include @@ -48,6 +51,21 @@ extern int max_threads; static struct workqueue_struct *khelper_wq; +#ifdef CONFIG_NAMESPACES +#define UMH_HASH_SHIFT 6 +#define UMH_HASH_SIZE 1 << UMH_HASH_SHIFT + +struct umh_ns_entry { + long token; + unsigned int count; + struct umh_ns_info nsinfo; + struct hlist_node umh_ns_hlist; +}; + +static DEFINE_SPINLOCK(umh_ns_hash_lock); +static struct hlist_head umh_ns_hash[UMH_HASH_SIZE]; +#endif + #define CAP_BSET (void *)1 #define CAP_PI (void *)2 @@ -577,10 +595,13 @@ static int umh_get_nsproxy(struct subprocess_info *sub_info) static void umh_put_nsproxy(struct subprocess_info *sub_info) { } + +static void umh_ns_hash_init(void) +{ +} #else -static int umh_get_nsproxy(struct subprocess_info *sub_info) +static int _umh_get_nsproxy(struct umh_ns_info *nsinfo) { - struct umh_ns_info *nsinfo = &sub_info->nsinfo; struct task_struct *tsk; struct user_namespace *user_ns; struct nsproxy *new; @@ -614,14 +635,165 @@ out: return err; } +static int umh_get_nsproxy(struct subprocess_info *sub_info) +{ + return _umh_get_nsproxy(&sub_info->nsinfo); +} + +static void _umh_put_nsproxy(struct umh_ns_info *nsinfo) +{ + if (nsinfo->nsproxy) { + put_nsproxy(nsinfo->nsproxy); + put_user_ns(nsinfo->user_ns); + } +} + static void umh_put_nsproxy(struct subprocess_info *sub_info) { - if (sub_info->nsinfo.nsproxy) { - put_nsproxy(sub_info->nsinfo.nsproxy); - put_user_ns(sub_info->nsinfo.user_ns); + return _umh_put_nsproxy(&sub_info->nsinfo); +} + +static void umh_ns_hash_init(void) +{ + int i; + + for (i = 0; i < UMH_HASH_SIZE; i++) + INIT_HLIST_HEAD(&umh_ns_hash[i]); +} + +static struct umh_ns_entry *__umh_ns_find_entry(long token) +{ + struct umh_ns_entry *this, *entry; + struct hlist_head *bucket; + unsigned int hash; + + hash = hash_64((unsigned long) token, UMH_HASH_SHIFT); + bucket = &umh_ns_hash[hash]; + + entry = ERR_PTR(-ENOENT); + if (hlist_empty(bucket)) + goto out; + + hlist_for_each_entry(this, bucket, umh_ns_hlist) { + if (this->token == token) { + entry = this; + break; + } } +out: + return entry; } -#endif + +static struct umh_ns_entry *umh_ns_find_entry(long token, unsigned int nowait) +{ + struct umh_ns_entry *entry; + unsigned long flags; + + if (nowait) + spin_lock_irqsave(&umh_ns_hash_lock, flags); + else + spin_lock(&umh_ns_hash_lock); + entry = __umh_ns_find_entry(token); + if (nowait) + spin_unlock_irqrestore(&umh_ns_hash_lock, flags); + else + spin_unlock(&umh_ns_hash_lock); + + return entry; +} + +/** + * umh_ns_get_token - allocate and store namespace information of the + * init process of the caller + * @token: token of stored namspace information or zero for a new + *token. + * + * Returns a token used to locate the namespace information for calls to + * call_usermode_helper_ns() calls. On failure returns a negative errno. + */ +long umh_ns_get_token(long token) +{ + struct umh_ns_entry *entry; + struct hlist_head *bucket; + unsigned int hash; + unsigned int new_token; + int err; + + if (token) { + spin
[RFC PATCH v4 00/12] Second attempt at contained helper execution
Here is another update to the attempt at contained helper execution. The main change is I've tried to incorporate Oleg's suggestions of directly constructing the namespaces rather than using the open/setns approach and the addition of a namespace hash store. I'm not particularly happy with this so far as there are a bunch of ref counted objects and I've almost certainly got that wrong. But also there are object lifetime problems, some I'm aware of and for sure others I'm not. Also there is the integrity of the thread runner process. I haven't performed a double fork on thread execution, it might be painful to implement, so the thread runner might end up with the wrong namespace setup if an error occurs. Anyway, I've decided to stop spinning my wheels with this and post an update in the hope that others can offer suggestions to help and, of course, point out things I've missed. The other change has been to the nfs and KEYS patches. I've introduced the ability to get a token that can be used to save namespace information for later execution and I've attempted to use that for persistent namespace execution, as was discussed previously. I'm not at all sure I've done this in a sensible way but the token does need to be accessible at helper execution time which is why I've done it this way. I definitely need advice here too. --- Ian Kent (12): nsproxy - make create_new_namespaces() non-static kmod - rename call_usermodehelper() flags parameter vfs - move mnt_namespace definition to linux/mount.h kmod - add namespace aware thread runner kmod - teach call_usermodehelper() to use a namespace kmod - add namespace info store kmod - add call_usermodehelper_ns() nfsd - use namespace if not executing in init namespace nfs - cache_lib use namespace if not executing in init namespace nfs - objlayout use namespace if not executing in init namespace KEYS - use correct memory allocation flag in call_usermodehelper_keys() KEYS: exec request-key within the requesting task's init namespace fs/mount.h | 12 - fs/nfs/cache_lib.c |7 + fs/nfs/objlayout/objlayout.c |7 + fs/nfsd/netns.h |3 fs/nfsd/nfs4recover.c| 48 +++- fs/nfsd/nfsctl.c |6 + include/linux/key.h |3 include/linux/kmod.h | 56 + include/linux/mount.h| 14 + include/linux/nsproxy.h |3 include/linux/sunrpc/cache.h |2 kernel/kmod.c| 465 -- kernel/nsproxy.c |2 net/sunrpc/cache.c |5 security/keys/gc.c |2 security/keys/key.c |4 security/keys/request_key.c | 39 +++- 17 files changed, 620 insertions(+), 58 deletions(-) -- Ian -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[RFC PATCH v4 03/12] vfs - move mnt_namespace definition to linux/mount.h
From: Ian Kent The mnt_namespace definition will be needed by the usermode helper contained execution implementation, move it to include/linux/mount.h. Signed-off-by: Ian Kent Cc: Benjamin Coddington Cc: Al Viro Cc: J. Bruce Fields Cc: David Howells Cc: Trond Myklebust Cc: Oleg Nesterov Cc: Eric W. Biederman Cc: Jeff Layton --- fs/mount.h| 12 include/linux/mount.h | 14 +- 2 files changed, 13 insertions(+), 13 deletions(-) diff --git a/fs/mount.h b/fs/mount.h index 6a61c2b..5b8423b 100644 --- a/fs/mount.h +++ b/fs/mount.h @@ -1,20 +1,8 @@ #include #include #include -#include #include -struct mnt_namespace { - atomic_tcount; - struct ns_commonns; - struct mount * root; - struct list_headlist; - struct user_namespace *user_ns; - u64 seq;/* Sequence number to prevent loops */ - wait_queue_head_t poll; - u64 event; -}; - struct mnt_pcp { int mnt_count; int mnt_writers; diff --git a/include/linux/mount.h b/include/linux/mount.h index c2c561d..39dbcdf 100644 --- a/include/linux/mount.h +++ b/include/linux/mount.h @@ -15,11 +15,12 @@ #include #include #include +#include +#include struct super_block; struct vfsmount; struct dentry; -struct mnt_namespace; #define MNT_NOSUID 0x01 #define MNT_NODEV 0x02 @@ -62,6 +63,17 @@ struct mnt_namespace; #define MNT_SYNC_UMOUNT0x200 #define MNT_MARKED 0x400 +struct mnt_namespace { + atomic_tcount; + struct ns_commonns; + struct mount * root; + struct list_headlist; + struct user_namespace *user_ns; + u64 seq;/* Sequence number to prevent loops */ + wait_queue_head_t poll; + u64 event; +}; + struct vfsmount { struct dentry *mnt_root;/* root of the mounted tree */ struct super_block *mnt_sb; /* pointer to superblock */ -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[RFC PATCH v4 01/12] nsproxy - make create_new_namespaces() non-static
From: Ian Kent create_new_namespaces() will be needed by usermodehelper namespace restricted execution. Signed-off-by: Ian Kent Cc: Benjamin Coddington Cc: Al Viro Cc: J. Bruce Fields Cc: David Howells Cc: Trond Myklebust Cc: Stanislav Kinsbursky Cc: Oleg Nesterov Cc: Eric W. Biederman --- include/linux/nsproxy.h |3 +++ kernel/nsproxy.c|2 +- 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/include/linux/nsproxy.h b/include/linux/nsproxy.h index 35fa08f..dfe7dda 100644 --- a/include/linux/nsproxy.h +++ b/include/linux/nsproxy.h @@ -62,6 +62,9 @@ extern struct nsproxy init_nsproxy; * */ +struct nsproxy *create_new_namespaces(unsigned long flags, + struct task_struct *tsk, struct user_namespace *user_ns, + struct fs_struct *new_fs); int copy_namespaces(unsigned long flags, struct task_struct *tsk); void exit_task_namespaces(struct task_struct *tsk); void switch_task_namespaces(struct task_struct *tsk, struct nsproxy *new); diff --git a/kernel/nsproxy.c b/kernel/nsproxy.c index 49746c8..48d5e4a 100644 --- a/kernel/nsproxy.c +++ b/kernel/nsproxy.c @@ -56,7 +56,7 @@ static inline struct nsproxy *create_nsproxy(void) * Return the newly created nsproxy. Do not attach this to the task, * leave it to the caller to do proper locking and attach it to task. */ -static struct nsproxy *create_new_namespaces(unsigned long flags, +struct nsproxy *create_new_namespaces(unsigned long flags, struct task_struct *tsk, struct user_namespace *user_ns, struct fs_struct *new_fs) { -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC PATCH 5/8] KEYS: exec request-key within the requesting task's init namespace
On Mon, 2015-02-23 at 17:22 -0800, Benjamin Coddington wrote: > On Tue, 24 Feb 2015, Ian Kent wrote: > > > On Mon, 2015-02-23 at 09:52 -0500, J. Bruce Fields wrote: > > > On Sat, Feb 21, 2015 at 11:58:58AM +0800, Ian Kent wrote: > > > > On Fri, 2015-02-20 at 14:05 -0500, J. Bruce Fields wrote: > > > > > On Fri, Feb 20, 2015 at 12:07:15PM -0600, Eric W. Biederman wrote: > > > > > > "J. Bruce Fields" writes: > > > > > > > > > > > > > On Fri, Feb 20, 2015 at 05:33:25PM +0800, Ian Kent wrote: > > > > > > > > > > > > >> The case of nfsd state-recovery might be similar but you'll need > > > > > > >> to help > > > > > > >> me out a bit with that too. > > > > > > > > > > > > > > Each network namespace can have its own virtual nfs server. > > > > > > > Servers can > > > > > > > be started and stopped independently per network namespace. We > > > > > > > decide > > > > > > > which server should handle an incoming rpc by looking at the > > > > > > > network > > > > > > > namespace associated with the socket that it arrived over. > > > > > > > > > > > > > > A server is started by the rpc.nfsd command writing a value into > > > > > > > a magic > > > > > > > file somewhere. > > > > > > > > > > > > nit. Unless I am completely turned around that file is on the nfsd > > > > > > filesystem, that lives in fs/nfsd/nfs.c. > > > > > > > > > > > > So I bevelive this really is a case of figuring out what we want the > > > > > > semantics to be for mount and propogating the information down from > > > > > > mount to where we call the user mode helpers. > > > > > > > > > > Oops, I agree. So when I said: > > > > > > > > > > The upcalls need to happen consistently in one context for a > > > > > given virtual nfs server, and that context should probably be > > > > > derived from rpc.nfsd's somehow. > > > > > > > > > > Instead of "rpc.nfsd's", I think I should have said "the mounter of > > > > > the nfsd filesystem". > > > > > > > > > > Which is already how we choose a net namespace: nfsd_mount and > > > > > nfsd_fill_super store the current net namespace in s_fs_info. (And > > > > > then > > > > > grep for "netns" to see the places where that's used.) > > > > > > > > This is going to be mostly a restatement of what's already been said, > > > > partly for me to refer back to later and partly to clarify and confirm > > > > what I need to do, so prepare to be bored. > > > > > > > > As a result of Oleg's recommendations and comments, the next version of > > > > the series will take a reference to an nsproxy and a user namespace > > > > (from the init process of the calling task, while it's still a child of > > > > that task), it won't carry around task structs. There are still a couple > > > > of questions with this so it's not quite there yet. > > > > > > > > We'll have to wait and see if what I've done is enough to remedy Oleg's > > > > concerns too. LOL, and then there's the question of how much I'll need > > > > to do to get it to actually work. > > > > > > > > The other difference is obtaining the context (now nsproxy and user > > > > namspace) has been taken entirely within the usermode helper. I think > > > > that's a good thing from the calling process isolation requirement. That > > > > may need to change again based on the discussion here. > > > > > > > > Now we're starting to look at actual usage it's worth keeping in mind > > > > that how to execute within required namespaces has to be sound before we > > > > tackle use cases that have requirements over this fundamental > > > > functionality. > > > > > > > > There are a couple of things to think about. > > > > > > > > One thing that's needed is how to work out if the UMH_USE_NS is needed > > > > and another i
Re: [RFC PATCH 5/8] KEYS: exec request-key within the requesting task's init namespace
On Mon, 2015-02-23 at 09:52 -0500, J. Bruce Fields wrote: > On Sat, Feb 21, 2015 at 11:58:58AM +0800, Ian Kent wrote: > > On Fri, 2015-02-20 at 14:05 -0500, J. Bruce Fields wrote: > > > On Fri, Feb 20, 2015 at 12:07:15PM -0600, Eric W. Biederman wrote: > > > > "J. Bruce Fields" writes: > > > > > > > > > On Fri, Feb 20, 2015 at 05:33:25PM +0800, Ian Kent wrote: > > > > > > > > >> The case of nfsd state-recovery might be similar but you'll need to > > > > >> help > > > > >> me out a bit with that too. > > > > > > > > > > Each network namespace can have its own virtual nfs server. Servers > > > > > can > > > > > be started and stopped independently per network namespace. We decide > > > > > which server should handle an incoming rpc by looking at the network > > > > > namespace associated with the socket that it arrived over. > > > > > > > > > > A server is started by the rpc.nfsd command writing a value into a > > > > > magic > > > > > file somewhere. > > > > > > > > nit. Unless I am completely turned around that file is on the nfsd > > > > filesystem, that lives in fs/nfsd/nfs.c. > > > > > > > > So I bevelive this really is a case of figuring out what we want the > > > > semantics to be for mount and propogating the information down from > > > > mount to where we call the user mode helpers. > > > > > > Oops, I agree. So when I said: > > > > > > The upcalls need to happen consistently in one context for a > > > given virtual nfs server, and that context should probably be > > > derived from rpc.nfsd's somehow. > > > > > > Instead of "rpc.nfsd's", I think I should have said "the mounter of > > > the nfsd filesystem". > > > > > > Which is already how we choose a net namespace: nfsd_mount and > > > nfsd_fill_super store the current net namespace in s_fs_info. (And then > > > grep for "netns" to see the places where that's used.) > > > > This is going to be mostly a restatement of what's already been said, > > partly for me to refer back to later and partly to clarify and confirm > > what I need to do, so prepare to be bored. > > > > As a result of Oleg's recommendations and comments, the next version of > > the series will take a reference to an nsproxy and a user namespace > > (from the init process of the calling task, while it's still a child of > > that task), it won't carry around task structs. There are still a couple > > of questions with this so it's not quite there yet. > > > > We'll have to wait and see if what I've done is enough to remedy Oleg's > > concerns too. LOL, and then there's the question of how much I'll need > > to do to get it to actually work. > > > > The other difference is obtaining the context (now nsproxy and user > > namspace) has been taken entirely within the usermode helper. I think > > that's a good thing from the calling process isolation requirement. That > > may need to change again based on the discussion here. > > > > Now we're starting to look at actual usage it's worth keeping in mind > > that how to execute within required namespaces has to be sound before we > > tackle use cases that have requirements over this fundamental > > functionality. > > > > There are a couple of things to think about. > > > > One thing that's needed is how to work out if the UMH_USE_NS is needed > > and another is how to provide provide persistent usage of particular > > namespaces across containers. The later probably will relate to the > > origin of the file system (which looks like it will be identified at > > mount time). > > > > The first case is when the mount originates in the root init namespace > > and most of the time (if not all the time) the UMH_USE_NS doesn't need > > to be set and the helper should run in the root init namspace. > > The helper always runs in the original mount's container. Sometimes > that container is the init container, yes, but I don't see what value > there is in setting a flag in that one case. Yep. that's pretty much what I meant. > > > That > > should work for mount propagation as well with mounts bound into a > > container. > > > > Is this also
Re: [RFC PATCH 5/8] KEYS: exec request-key within the requesting task's init namespace
On Fri, 2015-02-20 at 14:05 -0500, J. Bruce Fields wrote: > On Fri, Feb 20, 2015 at 12:07:15PM -0600, Eric W. Biederman wrote: > > "J. Bruce Fields" writes: > > > > > On Fri, Feb 20, 2015 at 05:33:25PM +0800, Ian Kent wrote: > > > > >> The case of nfsd state-recovery might be similar but you'll need to help > > >> me out a bit with that too. > > > > > > Each network namespace can have its own virtual nfs server. Servers can > > > be started and stopped independently per network namespace. We decide > > > which server should handle an incoming rpc by looking at the network > > > namespace associated with the socket that it arrived over. > > > > > > A server is started by the rpc.nfsd command writing a value into a magic > > > file somewhere. > > > > nit. Unless I am completely turned around that file is on the nfsd > > filesystem, that lives in fs/nfsd/nfs.c. > > > > So I bevelive this really is a case of figuring out what we want the > > semantics to be for mount and propogating the information down from > > mount to where we call the user mode helpers. > > Oops, I agree. So when I said: > > The upcalls need to happen consistently in one context for a > given virtual nfs server, and that context should probably be > derived from rpc.nfsd's somehow. > > Instead of "rpc.nfsd's", I think I should have said "the mounter of > the nfsd filesystem". > > Which is already how we choose a net namespace: nfsd_mount and > nfsd_fill_super store the current net namespace in s_fs_info. (And then > grep for "netns" to see the places where that's used.) This is going to be mostly a restatement of what's already been said, partly for me to refer back to later and partly to clarify and confirm what I need to do, so prepare to be bored. As a result of Oleg's recommendations and comments, the next version of the series will take a reference to an nsproxy and a user namespace (from the init process of the calling task, while it's still a child of that task), it won't carry around task structs. There are still a couple of questions with this so it's not quite there yet. We'll have to wait and see if what I've done is enough to remedy Oleg's concerns too. LOL, and then there's the question of how much I'll need to do to get it to actually work. The other difference is obtaining the context (now nsproxy and user namspace) has been taken entirely within the usermode helper. I think that's a good thing from the calling process isolation requirement. That may need to change again based on the discussion here. Now we're starting to look at actual usage it's worth keeping in mind that how to execute within required namespaces has to be sound before we tackle use cases that have requirements over this fundamental functionality. There are a couple of things to think about. One thing that's needed is how to work out if the UMH_USE_NS is needed and another is how to provide provide persistent usage of particular namespaces across containers. The later probably will relate to the origin of the file system (which looks like it will be identified at mount time). The first case is when the mount originates in the root init namespace and most of the time (if not all the time) the UMH_USE_NS doesn't need to be set and the helper should run in the root init namspace. That should work for mount propagation as well with mounts bound into a container. Is this also true for automounted mounts at mount point crossing? Or perhaps I should ask, should automounted NFS mounts inherit the property from their parent mount? The second case is when the mount originates in another namespace, possibly a container. TBH I haven't thought too much about mounts that originate from namespaces created by unshare(1) or other source yet. I'm hoping that will just work once this is done, ;) The last time I tried binding NFS mounts from one container into another it didn't work, but if we assume that will work at some point then, as Bruce points out, we need to provide the ability to record the namespaces to be used for subsequent "in namespace" execution while maintaining caller isolation (ie. derived from the callers init process). I've been aware of the need for persistence for a while now and I've been thinking about how to do it but I don't have a clear plan quite yet. Bruce, having noticed this, has described details about the environment I have to work with so that's a start. I need the thoughts of others on this too. As a result I'm not sure yet if this persistence can be integrated into the current implementation or if additional
Re: [RFC PATCH 5/8] KEYS: exec request-key within the requesting task's init namespace
On Wed, 2015-02-18 at 20:31 -0500, J. Bruce Fields wrote: > On Thu, Feb 19, 2015 at 08:39:01AM +0800, Ian Kent wrote: > > On Wed, 2015-02-18 at 15:59 -0500, J. Bruce Fields wrote: > > > On Wed, Feb 18, 2015 at 12:31:32PM -0500, J. Bruce Fields wrote: > > > > On Wed, Feb 18, 2015 at 12:06:20PM -0500, J. Bruce Fields wrote: > > > > > On Fri, Feb 06, 2015 at 09:47:25AM +0800, Ian Kent wrote: > > > > > > On Thu, 2015-02-05 at 15:14 +, David Howells wrote: > > > > > > > > > > > > > > > + /* If running within a container use the container > > > > > > > > namespace */ > > > > > > > > + if (current->nsproxy->net_ns != &init_net) > > > > > > > > > > > > > > Is that a viable check? Is it possible to have a container that > > > > > > > shares > > > > > > > networking details? > > > > > > > > > > > > That's up for discussion. > > > > > > > > > > > > I thought about it and concluded that the check is probably not > > > > > > sufficient for any of the cases. > > > > > > > > > > > > I left it like that because I'm not sure exactly what the use cases > > > > > > are, > > > > > > hoping it promote discussion and here we are. > > > > > > > > > > > > I also think the current container environments don't share net > > > > > > namespace with the root init net namspace, necessarily, because thy > > > > > > are > > > > > > containers, ;) > > > > > > > > > > > > TBH I haven't looked at the user space container creation code but I > > > > > > expect it could be done that way if it was needed, so the answer is > > > > > > yes > > > > > > and no, ;) > > > > > > > > > > > > The questions then are do we need to check anything else, and what > > > > > > environment should the callback use in the different cases, and what > > > > > > other cases might break if we change it? > > > > > > > > > > > > For example, should the fs namespace also be checked for all of > > > > > > these > > > > > > cases, since we're executing a callback, or is whatever that's set > > > > > > to in > > > > > > the container always what's required for locating the executable. > > > > > > > > > > What would be the disadvantage of setting UMH_USE_NS unconditionally > > > > > here? > > > > > > > > In the nfs idmapping case, the mapping is per-nfs_client. > > > > > > > > Can nfs_idmap_new be the one that calls umh_get_init_task, with the > > > > corresponding put done in nfs_idmap_delete, or is there some reason that > > > > doesn't work? > > > > > > It's confusing sorting out possible use cases, but I think both of these > > > are reasonable: > > > > > > - mount an nfs filesystem from the host, then spawn containers > > > that all use that filesystem. > > > - mount an nfs filesystem from within a container. > > > > > > Your approach might work for the second, but in the first case we'll end > > > up with idmappers from multiple containers all trying to do the > > > idmapping for the shared filesystem. > > > > These patches are examples for context. > > > > Working out whether to run in a namespace or not was always going to be > > difficult, specifically for the case you point out. Maybe we can make > > use of some other information, namespace information in the super block > > perhaps, or something else, or perhaps we will need to add some > > information for this, not sure yet. We'll need to work together on that. > > > > TBH, I'm not that focused on the use cases because the base > > implementation is still undergoing significant change although I believe > > the use of a flag to request namespace execution is a good approach. > > That probably won't change. > > The flag requests that we use the container of the currently executing > task. In neither the nfs idmapper nor the nfsd state-recovery case is > that the correct choice. There's a bit more to do but I've done the chang
Re: [RFC PATCH 5/8] KEYS: exec request-key within the requesting task's init namespace
On Wed, 2015-02-18 at 20:31 -0500, J. Bruce Fields wrote: > On Thu, Feb 19, 2015 at 08:39:01AM +0800, Ian Kent wrote: > > On Wed, 2015-02-18 at 15:59 -0500, J. Bruce Fields wrote: > > > On Wed, Feb 18, 2015 at 12:31:32PM -0500, J. Bruce Fields wrote: > > > > On Wed, Feb 18, 2015 at 12:06:20PM -0500, J. Bruce Fields wrote: > > > > > On Fri, Feb 06, 2015 at 09:47:25AM +0800, Ian Kent wrote: > > > > > > On Thu, 2015-02-05 at 15:14 +, David Howells wrote: > > > > > > > > > > > > > > > + /* If running within a container use the container > > > > > > > > namespace */ > > > > > > > > + if (current->nsproxy->net_ns != &init_net) > > > > > > > > > > > > > > Is that a viable check? Is it possible to have a container that > > > > > > > shares > > > > > > > networking details? > > > > > > > > > > > > That's up for discussion. > > > > > > > > > > > > I thought about it and concluded that the check is probably not > > > > > > sufficient for any of the cases. > > > > > > > > > > > > I left it like that because I'm not sure exactly what the use cases > > > > > > are, > > > > > > hoping it promote discussion and here we are. > > > > > > > > > > > > I also think the current container environments don't share net > > > > > > namespace with the root init net namspace, necessarily, because thy > > > > > > are > > > > > > containers, ;) > > > > > > > > > > > > TBH I haven't looked at the user space container creation code but I > > > > > > expect it could be done that way if it was needed, so the answer is > > > > > > yes > > > > > > and no, ;) > > > > > > > > > > > > The questions then are do we need to check anything else, and what > > > > > > environment should the callback use in the different cases, and what > > > > > > other cases might break if we change it? > > > > > > > > > > > > For example, should the fs namespace also be checked for all of > > > > > > these > > > > > > cases, since we're executing a callback, or is whatever that's set > > > > > > to in > > > > > > the container always what's required for locating the executable. > > > > > > > > > > What would be the disadvantage of setting UMH_USE_NS unconditionally > > > > > here? > > > > > > > > In the nfs idmapping case, the mapping is per-nfs_client. > > > > > > > > Can nfs_idmap_new be the one that calls umh_get_init_task, with the > > > > corresponding put done in nfs_idmap_delete, or is there some reason that > > > > doesn't work? > > > > > > It's confusing sorting out possible use cases, but I think both of these > > > are reasonable: > > > > > > - mount an nfs filesystem from the host, then spawn containers > > > that all use that filesystem. > > > - mount an nfs filesystem from within a container. > > > > > > Your approach might work for the second, but in the first case we'll end > > > up with idmappers from multiple containers all trying to do the > > > idmapping for the shared filesystem. > > > > These patches are examples for context. > > > > Working out whether to run in a namespace or not was always going to be > > difficult, specifically for the case you point out. Maybe we can make > > use of some other information, namespace information in the super block > > perhaps, or something else, or perhaps we will need to add some > > information for this, not sure yet. We'll need to work together on that. > > > > TBH, I'm not that focused on the use cases because the base > > implementation is still undergoing significant change although I believe > > the use of a flag to request namespace execution is a good approach. > > That probably won't change. > > The flag requests that we use the container of the currently executing > task. In neither the nfs idmapper nor the nfsd state-recovery case is > that the correct choice. OK, I'll drop those patches then. Ian -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC PATCH 5/8] KEYS: exec request-key within the requesting task's init namespace
On Wed, 2015-02-18 at 15:59 -0500, J. Bruce Fields wrote: > On Wed, Feb 18, 2015 at 12:31:32PM -0500, J. Bruce Fields wrote: > > On Wed, Feb 18, 2015 at 12:06:20PM -0500, J. Bruce Fields wrote: > > > On Fri, Feb 06, 2015 at 09:47:25AM +0800, Ian Kent wrote: > > > > On Thu, 2015-02-05 at 15:14 +, David Howells wrote: > > > > > > > > > > > + /* If running within a container use the container namespace */ > > > > > > + if (current->nsproxy->net_ns != &init_net) > > > > > > > > > > Is that a viable check? Is it possible to have a container that > > > > > shares > > > > > networking details? > > > > > > > > That's up for discussion. > > > > > > > > I thought about it and concluded that the check is probably not > > > > sufficient for any of the cases. > > > > > > > > I left it like that because I'm not sure exactly what the use cases are, > > > > hoping it promote discussion and here we are. > > > > > > > > I also think the current container environments don't share net > > > > namespace with the root init net namspace, necessarily, because thy are > > > > containers, ;) > > > > > > > > TBH I haven't looked at the user space container creation code but I > > > > expect it could be done that way if it was needed, so the answer is yes > > > > and no, ;) > > > > > > > > The questions then are do we need to check anything else, and what > > > > environment should the callback use in the different cases, and what > > > > other cases might break if we change it? > > > > > > > > For example, should the fs namespace also be checked for all of these > > > > cases, since we're executing a callback, or is whatever that's set to in > > > > the container always what's required for locating the executable. > > > > > > What would be the disadvantage of setting UMH_USE_NS unconditionally > > > here? > > > > In the nfs idmapping case, the mapping is per-nfs_client. > > > > Can nfs_idmap_new be the one that calls umh_get_init_task, with the > > corresponding put done in nfs_idmap_delete, or is there some reason that > > doesn't work? > > It's confusing sorting out possible use cases, but I think both of these > are reasonable: > > - mount an nfs filesystem from the host, then spawn containers > that all use that filesystem. > - mount an nfs filesystem from within a container. > > Your approach might work for the second, but in the first case we'll end > up with idmappers from multiple containers all trying to do the > idmapping for the shared filesystem. These patches are examples for context. Working out whether to run in a namespace or not was always going to be difficult, specifically for the case you point out. Maybe we can make use of some other information, namespace information in the super block perhaps, or something else, or perhaps we will need to add some information for this, not sure yet. We'll need to work together on that. TBH, I'm not that focused on the use cases because the base implementation is still undergoing significant change although I believe the use of a flag to request namespace execution is a good approach. That probably won't change. Ian -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC PATCH 3/8] kmod - teach call_usermodehelper() to use a namespace
On Mon, 2015-02-16 at 19:24 +0100, Oleg Nesterov wrote: > On 02/16, Oleg Nesterov wrote: > > > > On 02/16, Ian Kent wrote: > > > > > > On Tue, 2015-02-10 at 17:55 +0100, Oleg Nesterov wrote: > > > > On 02/10, Ian Kent wrote: > > > > > > > > > > On Mon, 2015-02-09 at 17:03 +0100, Oleg Nesterov wrote: > > > > > > > > > > > > I understand. but I still can't understand why we can't implement > > > > > > something > > > > > > like > > > > > > enter_ns(struct nsproxy *p) > > > > > > { > > > > > > new_nsproxy = create_new_namespaces(...); > > > > > > > > > > > > p->mnt_ns->ns->ops->install(new_nsproxy, ...); > > > > > > p->pid_ns_for_children->ns->ops->install(new_nsproxy, > > > > > > ...); > > > > > > ... > > > > > > > > > > > > switch_task_namespaces(new_nsproxy); > > > > > > } > > > > > > > > > > > > Why we should abuse fs/proc ? > > > > > > > > > > That sounds like a much better approach. > > > > > Your saying just take a reference to the nsproxy from the located > > > > > process and use it instead, right? > > > > > > > > Yes, > > > > > > I'm still not sure if this can be done (at least without surgery to the > > > namespace implementation) and I think I've been here before which is > > > what lead to the file_open_root() approach. > > > > > > The difficulty is the second parameter to the install() call above, the > > > struct ns_common. In setns() it's obtained from the procfs file inode > > > and the file open is what's used to obtain each namespace type (in the > > > form of a struct ns_common) from a process context different from > > > current, current being the thread runner process. > > > > > > I might still be able to work out a (viable) way to obtain the > > > appropriate ns_common struct in each case without a file open but it's > > > hard to see how. > > > > Not sure I understand... Every "namespace" pointer "struct nsproxy" has > > the "struct ns_common ns" you need? So you can do (for example) > > > > p->mnt_ns->ns->ops->install(new_nsproxy, &p->mnt_ns->ns); > > > > or I missed something? (userns differs, you need cred->user_ns, of course). > > > > > > Perhaps I missed something, but this doesn't look like a problem... > > And in case this wasn't clear we obviously need to pass nsproxy/cred to > sub_info->init(). > > There is nothing magical in /proc or I am totally confused. Look at > ns_get_path(), > it (to simplify) does > > inode->i_private = ns_ops->get(task); > > and every ->get() method returns &ns->ns. Yep, my confusion was with which nsproxy is the needed nsproxy and your right, there isn't anything special about setting the inode nsproxy. It looks like your comments expose the file open as a really complicated way of getting back the nsproxy(->ns) that we (will) already have from the located task. I should look a little further to confirm that but, in hindsight, it seems obvious now. And I think that's what your trying so hard to get through to me, ;) > > > But again, again: > > > The real problem is that , let me repeat, is that pidns_install() does not > > and can't change active_pid_ns. So I think that kernel_thread_in_ns() > > probably > > make more sense. > > Yes... Yes, fortunately I got that when you first mentioned it. Your point here was easy to verify and understand. Thanks for your persistence. Ian -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC PATCH 3/8] kmod - teach call_usermodehelper() to use a namespace
On Mon, 2015-02-16 at 18:13 +0100, Oleg Nesterov wrote: > On 02/16, Ian Kent wrote: > > > > On Tue, 2015-02-10 at 17:55 +0100, Oleg Nesterov wrote: > > > On 02/10, Ian Kent wrote: > > > > > > > > On Mon, 2015-02-09 at 17:03 +0100, Oleg Nesterov wrote: > > > > > > > > > > I understand. but I still can't understand why we can't implement > > > > > something > > > > > like > > > > > enter_ns(struct nsproxy *p) > > > > > { > > > > > new_nsproxy = create_new_namespaces(...); > > > > > > > > > > p->mnt_ns->ns->ops->install(new_nsproxy, ...); > > > > > p->pid_ns_for_children->ns->ops->install(new_nsproxy, > > > > > ...); > > > > > ... > > > > > > > > > > switch_task_namespaces(new_nsproxy); > > > > > } > > > > > > > > > > Why we should abuse fs/proc ? > > > > > > > > That sounds like a much better approach. > > > > Your saying just take a reference to the nsproxy from the located > > > > process and use it instead, right? > > > > > > Yes, > > > > I'm still not sure if this can be done (at least without surgery to the > > namespace implementation) and I think I've been here before which is > > what lead to the file_open_root() approach. > > > > The difficulty is the second parameter to the install() call above, the > > struct ns_common. In setns() it's obtained from the procfs file inode > > and the file open is what's used to obtain each namespace type (in the > > form of a struct ns_common) from a process context different from > > current, current being the thread runner process. > > > > I might still be able to work out a (viable) way to obtain the > > appropriate ns_common struct in each case without a file open but it's > > hard to see how. > > Not sure I understand... Every "namespace" pointer "struct nsproxy" has > the "struct ns_common ns" you need? So you can do (for example) > > p->mnt_ns->ns->ops->install(new_nsproxy, &p->mnt_ns->ns); > > or I missed something? (userns differs, you need cred->user_ns, of course). I didn't see that when I looked so I missed it, thanks for pointing it out. > > > Perhaps I missed something, but this doesn't look like a problem... > > The real problem is that , let me repeat, is that pidns_install() does not > and can't change active_pid_ns. So I think that kernel_thread_in_ns() probably > make more sense. Right, I didn't miss that when you mentioned it. Changing the execution order using your kernel_thread_in_ns() suggestion is clearly what needs to be done. Ian -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC PATCH 3/8] kmod - teach call_usermodehelper() to use a namespace
On Tue, 2015-02-10 at 17:55 +0100, Oleg Nesterov wrote: > On 02/10, Ian Kent wrote: > > > > On Mon, 2015-02-09 at 17:03 +0100, Oleg Nesterov wrote: > > > > > > I understand. but I still can't understand why we can't implement > > > something > > > like > > > enter_ns(struct nsproxy *p) > > > { > > > new_nsproxy = create_new_namespaces(...); > > > > > > p->mnt_ns->ns->ops->install(new_nsproxy, ...); > > > p->pid_ns_for_children->ns->ops->install(new_nsproxy, ...); > > > ... > > > > > > switch_task_namespaces(new_nsproxy); > > > } > > > > > > Why we should abuse fs/proc ? > > > > That sounds like a much better approach. > > Your saying just take a reference to the nsproxy from the located > > process and use it instead, right? > > Yes, I'm still not sure if this can be done (at least without surgery to the namespace implementation) and I think I've been here before which is what lead to the file_open_root() approach. The difficulty is the second parameter to the install() call above, the struct ns_common. In setns() it's obtained from the procfs file inode and the file open is what's used to obtain each namespace type (in the form of a struct ns_common) from a process context different from current, current being the thread runner process. I might still be able to work out a (viable) way to obtain the appropriate ns_common struct in each case without a file open but it's hard to see how. Ian -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] autofs4: Wrong format for printing dentry
On Wed, 2015-02-11 at 21:41 +0100, Rasmus Villemoes wrote: > Ping... The patch looks fine to me. I'll check there are no other instances of this and send it to Al. > > On Fri, Feb 06 2015, Rasmus Villemoes wrote: > > > %pD for struct file*, %pd for struct dentry*. > > > > Fixes: a455589f181e ("assorted conversions to %p[dD]") > > Signed-off-by: Rasmus Villemoes > > --- > > fs/autofs4/root.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/fs/autofs4/root.c b/fs/autofs4/root.c > > index dbb5b7212ce1..7ba355b8d4ac 100644 > > --- a/fs/autofs4/root.c > > +++ b/fs/autofs4/root.c > > @@ -108,7 +108,7 @@ static int autofs4_dir_open(struct inode *inode, struct > > file *file) > > struct dentry *dentry = file->f_path.dentry; > > struct autofs_sb_info *sbi = autofs4_sbi(dentry->d_sb); > > > > - DPRINTK("file=%p dentry=%p %pD", file, dentry, dentry); > > + DPRINTK("file=%p dentry=%p %pd", file, dentry, dentry); > > > > if (autofs4_oz_mode(sbi)) > > goto out; -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC PATCH 3/8] kmod - teach call_usermodehelper() to use a namespace
On Tue, 2015-02-10 at 17:55 +0100, Oleg Nesterov wrote: > On 02/10, Ian Kent wrote: > > > > On Mon, 2015-02-09 at 17:03 +0100, Oleg Nesterov wrote: > > > > > > I understand. but I still can't understand why we can't implement > > > something > > > like > > > enter_ns(struct nsproxy *p) > > > { > > > new_nsproxy = create_new_namespaces(...); > > > > > > p->mnt_ns->ns->ops->install(new_nsproxy, ...); > > > p->pid_ns_for_children->ns->ops->install(new_nsproxy, ...); > > > ... > > > > > > switch_task_namespaces(new_nsproxy); > > > } > > > > > > Why we should abuse fs/proc ? > > > > That sounds like a much better approach. > > Your saying just take a reference to the nsproxy from the located > > process and use it instead, right? > > Yes, > > > Working out if there's a difference with what you from the open is > > challenging (I already tried), I'll have another go at it. > > I thinks there should not be any difference, but please re-check ;) > > > > And. Whatever we do, ops->install() or setns_inode() can't solve the > > > problem with > > > pid_ns. You need the additional clone() to "activate" it. pidns_install() > > > does not > > > actually change task_active_pid_ns(). > > > > Right, but all this is done in preparation for the following do_execve() > > call. Isn't that enough or am I missing something? > > Yes, but do_execve() doesn't (and shouldn't) change task_active_pid_ns(). Note > the ->pid_ns_for_children's name. It is only used by > copy_process()->alloc_pid(). Right, I vaguely recall seeing something like this earlier in the fork procedure, I get it. I'll need to have a another look at it. > > task_active_pid_ns() uses task_pid() and we obviously can't change it. > > I am wondering if we can do something like > > kernel_thread_in_ns(struct nsproxy *ns, ...) > { > struct nsproxy *saved_ns = current->nsproxy; > pid_t pid; > > task_lock(current); > current->nsproxy = ns; > task_unlock(current); > > pid = kernel_thread(...); > > task_lock(current); > current->nsproxy = saved_ns; > task_unlock(current); > > return pid; > } > > used by __call_usermodehelper/wait_for_helper, instead of "enter_ns" from > sub_info->init()... Again, thanks for the suggestions. You've given me a few things think about and check out. Ian -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC PATCH 3/8] kmod - teach call_usermodehelper() to use a namespace
On Mon, 2015-02-09 at 17:03 +0100, Oleg Nesterov wrote: > On 02/09, Ian Kent wrote: > > > > On Sun, 2015-02-08 at 20:00 +0100, Oleg Nesterov wrote: > > > > + > > > > + this = file_open_root(mnt->mnt_root, mnt, path, > > > > O_RDONLY); > > > > + if (unlikely(IS_ERR(this))) { > > > > + err = PTR_ERR(this); > > > > + break; > > > > + } > > > > + > > > > + err = setns_inode(file_inode(this), 0); > > > > + fput(this); > > > > + if (err) > > > > + break; > > > > + } > > > > + > > > > + return err; > > > > +} > > > > > > Yes, I need to actually read this series and setns paths, but at first > > > glance > > > there must be a simpler method to call ops->install's and > > > switch_task_namespaces. > > > > Yes, the namespaces implementation does seem a bit strange in this > > respect. I mentioned that concern the first time I posted these. But I'm > > still not that clear on the big picture of how namespace are meant to > > work. > > > > It's not just access to ops->install() that's the problem. > > > > For each of the individual namespaces we open a file handle, to get > > access to ops->install() for that namespace, install it, drop "all" the > > namespaces then replace them with the new set that essentially has one > > namespace changed. > > I understand. but I still can't understand why we can't implement something > like > enter_ns(struct nsproxy *p) > { > new_nsproxy = create_new_namespaces(...); > > p->mnt_ns->ns->ops->install(new_nsproxy, ...); > p->pid_ns_for_children->ns->ops->install(new_nsproxy, ...); > ... > > switch_task_namespaces(new_nsproxy); > } > > Why we should abuse fs/proc ? That sounds like a much better approach. Your saying just take a reference to the nsproxy from the located process and use it instead, right? Working out if there's a difference with what you from the open is challenging (I already tried), I'll have another go at it. > > See also below. > > > > Sorry if this was already discussed before, but to me it looks a bit > > > strange > > > to abuse /proc/ files for this. And again, iiuc file_open_root() can fail > > > if > > > tsk has already exited (init can be multithreaded). > > > > Not sure that the failure is a problem though as long as it's handled > > since, if the init process of the container is gone (or will be gone > > once were done), so is the container and the caller. > > Not really. Individual thread can exit while the whole "init" process can be > alive. > In particular the main thread can exit and become a zombie, so > find_task_by_vpid(1) > can't work in general. > > You can probably use task_active_pid_ns()-child_reaper, but again I do not > think > you should pass "task_struct *" to enter_ns(). OK, I need to check this, btw, thanks for the comments. > > > And. Whatever we do, ops->install() or setns_inode() can't solve the problem > with > pid_ns. You need the additional clone() to "activate" it. pidns_install() > does not > actually change task_active_pid_ns(). Right, but all this is done in preparation for the following do_execve() call. Isn't that enough or am I missing something? Ian -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC PATCH 3/8] kmod - teach call_usermodehelper() to use a namespace
On Sun, 2015-02-08 at 20:00 +0100, Oleg Nesterov wrote: > On 02/05, Ian Kent wrote: > > > > +int umh_enter_ns(struct task_struct *tsk, struct cred *new) > > +{ > > + char path[NS_PATH_MAX]; > > + struct vfsmount *mnt; > > + const char *name; > > + pid_t pid; > > + int err = 0; > > + > > + pid = task_pid_nr(tsk); > > + > > + /* > > +* The user mode thread runner runs in the root init namespace > > +* so it will see all system pids. > > +*/ > > + mnt = task_active_pid_ns(current)->proc_mnt; > > + > > + for (name = ns_names[0]; *name; name++) { > > + struct file *this; > > + int len; > > + > > + len = snprintf(path, > > + NS_PATH_MAX, NS_PATH_FMT, > > + (unsigned long) pid, name); > > + if (len >= NS_PATH_MAX) { > > + err = -ENAMETOOLONG; > > + break; > > + } > > + > > + this = file_open_root(mnt->mnt_root, mnt, path, O_RDONLY); > > + if (unlikely(IS_ERR(this))) { > > + err = PTR_ERR(this); > > + break; > > + } > > + > > + err = setns_inode(file_inode(this), 0); > > + fput(this); > > + if (err) > > + break; > > + } > > + > > + return err; > > +} > > Yes, I need to actually read this series and setns paths, but at first glance > there must be a simpler method to call ops->install's and > switch_task_namespaces. Yes, the namespaces implementation does seem a bit strange in this respect. I mentioned that concern the first time I posted these. But I'm still not that clear on the big picture of how namespace are meant to work. It's not just access to ops->install() that's the problem. For each of the individual namespaces we open a file handle, to get access to ops->install() for that namespace, install it, drop "all" the namespaces then replace them with the new set that essentially has one namespace changed. > > Sorry if this was already discussed before, but to me it looks a bit strange > to abuse /proc/ files for this. And again, iiuc file_open_root() can fail if > tsk has already exited (init can be multithreaded). Not sure that the failure is a problem though as long as it's handled since, if the init process of the container is gone (or will be gone once were done), so is the container and the caller. The use of proc is largely because we can't use the callers environment to setup the process as the caller could manipulate it to subvert the system. When not executing in a container the thread runner runs under root init so nothing needs to be done but in a container we want to use the init process of the container so the container's namespaces are used. There is probably a better way to do it, suggestions welcome! Ian -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC PATCH 3/8] kmod - teach call_usermodehelper() to use a namespace
On Fri, 2015-02-06 at 07:08 -0500, Jeff Layton wrote: > On Thu, 05 Feb 2015 10:34:11 +0800 > Ian Kent wrote: > > > The call_usermodehelper() function executes all binaries in the > > global "init" root context. This doesn't allow a binary to be run > > within a namespace (eg. the namespace of a container). > > > > Both containerized NFS client and NFS server need the ability to > > execute a binary in a container's context. To do this use the init > > process of the callers environment is used to setup the namespaces > > in the same way the root init process is used otherwise. > > > > Signed-off-by: Ian Kent > > Cc: Benjamin Coddington > > Cc: Al Viro > > Cc: J. Bruce Fields > > Cc: David Howells > > Cc: Trond Myklebust > > Cc: Oleg Nesterov > > Cc: Eric W. Biederman > > Cc: Jeff Layton > > --- > > include/linux/kmod.h | 16 +++ > > kernel/kmod.c| 115 > > +- > > 2 files changed, 128 insertions(+), 3 deletions(-) > > > > diff --git a/include/linux/kmod.h b/include/linux/kmod.h > > index 15bdeed..b0f1b3c 100644 > > --- a/include/linux/kmod.h > > +++ b/include/linux/kmod.h > > @@ -52,6 +52,7 @@ struct file; > > #define UMH_WAIT_EXEC 1 /* wait for the exec, but not the > > process */ > > #define UMH_WAIT_PROC 2 /* wait for the process to complete */ > > #define UMH_KILLABLE 4 /* wait for EXEC/PROC killable */ > > +#define UMH_USE_NS 8 /* exec using caller's init namespace */ > > > > struct subprocess_info { > > struct work_struct work; > > @@ -69,6 +70,21 @@ struct subprocess_info { > > extern int > > call_usermodehelper(char *path, char **argv, char **envp, int flags); > > > > +#if !defined(CONFIG_PROC_FS) || !defined(CONFIG_NAMESPACES) > > +inline struct task_struct *umh_get_init_task(void) > > +{ > > + return ERR_PTR(-ENOTSUP); > > +} > > + > > +inline int umh_enter_ns(struct task_struct *tsk, struct cred *new) > > +{ > > + return -ENOTSUP; > > +} > > +#else > > +struct task_struct *umh_get_init_pid(void); > > +int umh_enter_ns(struct task_struct *tsk, struct cred *new); > > +#endif > > + > > extern struct subprocess_info * > > call_usermodehelper_setup(char *path, char **argv, char **envp, gfp_t > > gfp_mask, > > int (*init)(struct subprocess_info *info, struct cred > > *new), > > diff --git a/kernel/kmod.c b/kernel/kmod.c > > index 14c0188..4c649d6 100644 > > --- a/kernel/kmod.c > > +++ b/kernel/kmod.c > > @@ -582,6 +582,98 @@ unlock: > > } > > EXPORT_SYMBOL(call_usermodehelper_exec); > > > > +#if defined(CONFIG_PROC_FS) && defined(CONFIG_NAMESPACES) > > +#define NS_PATH_MAX35 > > +#define NS_PATH_FMT"%lu/ns/%s" > > + > > +/* Note namespace name order is significant */ > > +static const char *ns_names[] = { "user", "ipc", "uts", "net", "pid", > > "mnt", NULL }; > > + > > +struct task_struct *umh_get_init_pid(void) > > nit: we're not getting a pid here but a task_struct pointer. Maybe this > should be called umh_get_init_task? Ha, yep. > > > +{ > > + struct task_struct *tsk; > > + > > + rcu_read_lock(); > > + tsk = find_task_by_vpid(1); > > + if (tsk) > > + get_task_struct(tsk); > > + rcu_read_unlock(); > > I'm not terribly familiar with the task_struct lifetime rules... > > I assume that you can be assured that tsk won't go away while you hold > the rcu_read_lock, but is doing a get_task_struct while holding it > sufficient to pin it after you drop the lock? > > IOW, could the refcount on the task_struct do a 0->1 transition here and > end up being freed anyway after you've grabbed a reference? Good point, I thought getting a reference under he read lock would be enough but maybe I need more checks as I do with dentrys. I'll check that. Ian -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC PATCH 5/8] KEYS: exec request-key within the requesting task's init namespace
On Thu, 2015-02-05 at 15:14 +, David Howells wrote: > > > + /* If running within a container use the container namespace */ > > + if (current->nsproxy->net_ns != &init_net) > > Is that a viable check? Is it possible to have a container that shares > networking details? That's up for discussion. I thought about it and concluded that the check is probably not sufficient for any of the cases. I left it like that because I'm not sure exactly what the use cases are, hoping it promote discussion and here we are. I also think the current container environments don't share net namespace with the root init net namspace, necessarily, because thy are containers, ;) TBH I haven't looked at the user space container creation code but I expect it could be done that way if it was needed, so the answer is yes and no, ;) The questions then are do we need to check anything else, and what environment should the callback use in the different cases, and what other cases might break if we change it? For example, should the fs namespace also be checked for all of these cases, since we're executing a callback, or is whatever that's set to in the container always what's required for locating the executable. Ian -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC PATCH 2/8] kmod - rename call_usermodehelper() flags parameter
On Thu, 2015-02-05 at 15:01 +, David Howells wrote: > Ian Kent wrote: > > > -call_usermodehelper(char *path, char **argv, char **envp, int wait); > > +call_usermodehelper(char *path, char **argv, char **envp, int flags); > > Can we make flags unsigned whilst we're at it? Other than that: Sure, thanks for your comments, here and elsewhere, I'll get onto fixing them. > > Acked-by: David Howells -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[RFC PATCH 8/8] nfs - objlayout use namespace if not executing in init namespace
If the caller is running within a container then execute the usermode helper callback within the init namespace of the container. Signed-off-by: Ian Kent Cc: Benjamin Coddington Cc: Al Viro Cc: J. Bruce Fields Cc: David Howells Cc: Trond Myklebust Cc: Oleg Nesterov Cc: Eric W. Biederman Cc: Jeff Layton --- fs/nfs/objlayout/objlayout.c |7 ++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/fs/nfs/objlayout/objlayout.c b/fs/nfs/objlayout/objlayout.c index 919efd4..dd5b9e8 100644 --- a/fs/nfs/objlayout/objlayout.c +++ b/fs/nfs/objlayout/objlayout.c @@ -600,8 +600,13 @@ static int __objlayout_upcall(struct __auto_login *login) NULL }; char *argv[8]; + int umh_flags = UMH_WAIT_PROC; int ret; + /* If running within a container use the container namespace */ + if (current->nsproxy->net_ns != &init_net) + umh_flags |= UMH_USE_NS; + if (unlikely(!osd_login_prog[0])) { dprintk("%s: osd_login_prog is disabled\n", __func__); return -EACCES; @@ -620,7 +625,7 @@ static int __objlayout_upcall(struct __auto_login *login) argv[6] = login->systemid_hex; argv[7] = NULL; - ret = call_usermodehelper(argv[0], argv, envp, UMH_WAIT_PROC); + ret = call_usermodehelper(argv[0], argv, envp, umh_flags); /* * Disable the upcall mechanism if we're getting an ENOENT or * EACCES error. The admin can re-enable it on the fly by using -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[RFC PATCH 0/8] v3 contained usermode helper execution
There haven't been any comments about the previous series not being an acceptable approach. Perhaps people were away, didn't notice or didn't have time. So here's another chance to speak up. In summary it's assumed that, since the usermode helper uses the root init namespace for process creation, using the init namespace of a container is eqivalent and sufficient when execution within a container is needed. Thinking further about callers I believe there are cases that won't be handled properly so I've tried to work out what the current use cases are and added patches that demonstrate simple minded usage. I'm not sure at all that this is sufficient so I need feedback. I've changed the execution to pin the calling task for the duration of the call as recommended by Jeff Layton but other than that not a lot has changed in the call back code. It's also not clear if the request key infrastructure will continue to use a usermode callback so we'll need to wait on that. --- Ian Kent (8): nsproxy - refactor setns() kmod - rename call_usermodehelper() flags parameter kmod - teach call_usermodehelper() to use a namespace KEYS - rename call_usermodehelper_keys() flags parameter KEYS: exec request-key within the requesting task's init namespace nfsd - use namespace if not executing in init namespace nfs - cache_lib use namespace if not executing in init namespace nfs - objlayout use namespace if not executing in init namespace fs/nfs/cache_lib.c |6 ++ fs/nfs/objlayout/objlayout.c |7 ++ fs/nfsd/netns.h |2 + fs/nfsd/nfs4recover.c| 48 ++- include/linux/kmod.h | 20 ++ include/linux/nsproxy.h |1 kernel/kmod.c| 131 ++ kernel/nsproxy.c | 21 --- security/keys/request_key.c | 64 + 9 files changed, 252 insertions(+), 48 deletions(-) -- Ian -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[RFC PATCH 2/8] kmod - rename call_usermodehelper() flags parameter
The wait parameter of call_usermodehelper() is not quite a parameter that describes the wait behaviour alone and will later be used to request exec within a namespace. So change its name to flags. Signed-off-by: Ian Kent Cc: Benjamin Coddington Cc: Al Viro Cc: J. Bruce Fields Cc: David Howells Cc: Trond Myklebust Cc: Oleg Nesterov Cc: Eric W. Biederman Cc: Jeff Layton --- include/linux/kmod.h |4 ++-- kernel/kmod.c| 16 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/include/linux/kmod.h b/include/linux/kmod.h index 0555cc6..15bdeed 100644 --- a/include/linux/kmod.h +++ b/include/linux/kmod.h @@ -67,7 +67,7 @@ struct subprocess_info { }; extern int -call_usermodehelper(char *path, char **argv, char **envp, int wait); +call_usermodehelper(char *path, char **argv, char **envp, int flags); extern struct subprocess_info * call_usermodehelper_setup(char *path, char **argv, char **envp, gfp_t gfp_mask, @@ -75,7 +75,7 @@ call_usermodehelper_setup(char *path, char **argv, char **envp, gfp_t gfp_mask, void (*cleanup)(struct subprocess_info *), void *data); extern int -call_usermodehelper_exec(struct subprocess_info *info, int wait); +call_usermodehelper_exec(struct subprocess_info *info, int flags); extern struct ctl_table usermodehelper_table[]; diff --git a/kernel/kmod.c b/kernel/kmod.c index 2777f40..14c0188 100644 --- a/kernel/kmod.c +++ b/kernel/kmod.c @@ -534,7 +534,7 @@ EXPORT_SYMBOL(call_usermodehelper_setup); * asynchronously if wait is not set, and runs as a child of keventd. * (ie. it runs with full root capabilities). */ -int call_usermodehelper_exec(struct subprocess_info *sub_info, int wait) +int call_usermodehelper_exec(struct subprocess_info *sub_info, int flags) { DECLARE_COMPLETION_ONSTACK(done); int retval = 0; @@ -553,14 +553,14 @@ int call_usermodehelper_exec(struct subprocess_info *sub_info, int wait) * This makes it possible to use umh_complete to free * the data structure in case of UMH_NO_WAIT. */ - sub_info->complete = (wait == UMH_NO_WAIT) ? NULL : &done; - sub_info->wait = wait; + sub_info->complete = (flags == UMH_NO_WAIT) ? NULL : &done; + sub_info->wait = flags; queue_work(khelper_wq, &sub_info->work); - if (wait == UMH_NO_WAIT)/* task has freed sub_info */ + if (flags == UMH_NO_WAIT) /* task has freed sub_info */ goto unlock; - if (wait & UMH_KILLABLE) { + if (flags & UMH_KILLABLE) { retval = wait_for_completion_killable(&done); if (!retval) goto wait_done; @@ -595,17 +595,17 @@ EXPORT_SYMBOL(call_usermodehelper_exec); * This function is the equivalent to use call_usermodehelper_setup() and * call_usermodehelper_exec(). */ -int call_usermodehelper(char *path, char **argv, char **envp, int wait) +int call_usermodehelper(char *path, char **argv, char **envp, int flags) { struct subprocess_info *info; - gfp_t gfp_mask = (wait == UMH_NO_WAIT) ? GFP_ATOMIC : GFP_KERNEL; + gfp_t gfp_mask = (flags == UMH_NO_WAIT) ? GFP_ATOMIC : GFP_KERNEL; info = call_usermodehelper_setup(path, argv, envp, gfp_mask, NULL, NULL, NULL); if (info == NULL) return -ENOMEM; - return call_usermodehelper_exec(info, wait); + return call_usermodehelper_exec(info, flags); } EXPORT_SYMBOL(call_usermodehelper); -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[RFC PATCH 5/8] KEYS: exec request-key within the requesting task's init namespace
Containerized request key helper callbacks need the ability to execute a binary in a container's context. To do this calling an in kernel equivalent of setns(2) should be sufficient since the user mode helper execution kernel thread ultimately calls do_execve(). Signed-off-by: Ian Kent Cc: Benjamin Coddington Cc: Al Viro Cc: J. Bruce Fields Cc: David Howells Cc: Trond Myklebust Cc: Oleg Nesterov Cc: Eric W. Biederman Cc: Jeff Layton --- security/keys/request_key.c | 60 +-- 1 file changed, 52 insertions(+), 8 deletions(-) diff --git a/security/keys/request_key.c b/security/keys/request_key.c index 9e79bbf..59282aa 100644 --- a/security/keys/request_key.c +++ b/security/keys/request_key.c @@ -17,6 +17,9 @@ #include #include #include +#include +#include +#include #include "internal.h" #define key_negative_timeout 60 /* default timeout on a negative key's existence */ @@ -46,6 +49,11 @@ void complete_request_key(struct key_construction *cons, int error) } EXPORT_SYMBOL(complete_request_key); +struct request_key_info { + struct key *keyring; + struct task_struct *tsk; +}; + /* * Initialise a usermode helper that is going to have a specific session * keyring. @@ -55,9 +63,18 @@ EXPORT_SYMBOL(complete_request_key); */ static int umh_keys_init(struct subprocess_info *info, struct cred *cred) { - struct key *keyring = info->data; + struct request_key_info *rki = info->data; + struct task_struct *tsk = rki->tsk; + + if (tsk) { + int err; + + err = umh_enter_ns(tsk, cred); + if (err) + return err; + } - return install_session_keyring_to_cred(cred, keyring); + return install_session_keyring_to_cred(cred, rki->keyring); } /* @@ -65,8 +82,11 @@ static int umh_keys_init(struct subprocess_info *info, struct cred *cred) */ static void umh_keys_cleanup(struct subprocess_info *info) { - struct key *keyring = info->data; - key_put(keyring); + struct request_key_info *rki = info->data; + if (rki->tsk) + put_task_struct(rki->tsk); + key_put(rki->keyring); + kfree(rki); } /* @@ -76,12 +96,32 @@ static int call_usermodehelper_keys(char *path, char **argv, char **envp, struct key *session_keyring, int flags) { struct subprocess_info *info; + struct request_key_info *rki; + unsigned int use_ns = flags & UMH_USE_NS; + struct task_struct *tsk = NULL; + + rki = kmalloc(sizeof(*rki), GFP_KERNEL); + if (!rki) + return -ENOMEM; + + if (use_ns) { + tsk = umh_get_init_pid(); + if (IS_ERR(tsk)) + return PTR_ERR(tsk); + } + + rki->keyring = session_keyring; + rki->tsk = tsk; info = call_usermodehelper_setup(path, argv, envp, GFP_KERNEL, umh_keys_init, umh_keys_cleanup, - session_keyring); - if (!info) + rki); + if (!info) { + if (tsk) + put_task_struct(rki->tsk); + kfree(rki); return -ENOMEM; + } key_get(session_keyring); return call_usermodehelper_exec(info, flags); @@ -102,10 +142,15 @@ static int call_sbin_request_key(struct key_construction *cons, char *argv[9], *envp[3], uid_str[12], gid_str[12]; char key_str[12], keyring_str[3][12]; char desc[20]; + int flags = UMH_WAIT_PROC; int ret, i; kenter("{%d},{%d},%s", key->serial, authkey->serial, op); + /* If running within a container use the container namespace */ + if (current->nsproxy->net_ns != &init_net) + flags |= UMH_USE_NS; + ret = install_user_keyrings(); if (ret < 0) goto error_alloc; @@ -172,8 +217,7 @@ static int call_sbin_request_key(struct key_construction *cons, argv[i] = NULL; /* do it */ - ret = call_usermodehelper_keys(argv[0], argv, envp, keyring, - UMH_WAIT_PROC); + ret = call_usermodehelper_keys(argv[0], argv, envp, keyring, flags); kdebug("usermode -> 0x%x", ret); if (ret >= 0) { /* ret is the exit/wait code */ -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[RFC PATCH 3/8] kmod - teach call_usermodehelper() to use a namespace
The call_usermodehelper() function executes all binaries in the global "init" root context. This doesn't allow a binary to be run within a namespace (eg. the namespace of a container). Both containerized NFS client and NFS server need the ability to execute a binary in a container's context. To do this use the init process of the callers environment is used to setup the namespaces in the same way the root init process is used otherwise. Signed-off-by: Ian Kent Cc: Benjamin Coddington Cc: Al Viro Cc: J. Bruce Fields Cc: David Howells Cc: Trond Myklebust Cc: Oleg Nesterov Cc: Eric W. Biederman Cc: Jeff Layton --- include/linux/kmod.h | 16 +++ kernel/kmod.c| 115 +- 2 files changed, 128 insertions(+), 3 deletions(-) diff --git a/include/linux/kmod.h b/include/linux/kmod.h index 15bdeed..b0f1b3c 100644 --- a/include/linux/kmod.h +++ b/include/linux/kmod.h @@ -52,6 +52,7 @@ struct file; #define UMH_WAIT_EXEC 1 /* wait for the exec, but not the process */ #define UMH_WAIT_PROC 2 /* wait for the process to complete */ #define UMH_KILLABLE 4 /* wait for EXEC/PROC killable */ +#define UMH_USE_NS 8 /* exec using caller's init namespace */ struct subprocess_info { struct work_struct work; @@ -69,6 +70,21 @@ struct subprocess_info { extern int call_usermodehelper(char *path, char **argv, char **envp, int flags); +#if !defined(CONFIG_PROC_FS) || !defined(CONFIG_NAMESPACES) +inline struct task_struct *umh_get_init_task(void) +{ + return ERR_PTR(-ENOTSUP); +} + +inline int umh_enter_ns(struct task_struct *tsk, struct cred *new) +{ + return -ENOTSUP; +} +#else +struct task_struct *umh_get_init_pid(void); +int umh_enter_ns(struct task_struct *tsk, struct cred *new); +#endif + extern struct subprocess_info * call_usermodehelper_setup(char *path, char **argv, char **envp, gfp_t gfp_mask, int (*init)(struct subprocess_info *info, struct cred *new), diff --git a/kernel/kmod.c b/kernel/kmod.c index 14c0188..4c649d6 100644 --- a/kernel/kmod.c +++ b/kernel/kmod.c @@ -582,6 +582,98 @@ unlock: } EXPORT_SYMBOL(call_usermodehelper_exec); +#if defined(CONFIG_PROC_FS) && defined(CONFIG_NAMESPACES) +#define NS_PATH_MAX35 +#define NS_PATH_FMT"%lu/ns/%s" + +/* Note namespace name order is significant */ +static const char *ns_names[] = { "user", "ipc", "uts", "net", "pid", "mnt", NULL }; + +struct task_struct *umh_get_init_pid(void) +{ + struct task_struct *tsk; + + rcu_read_lock(); + tsk = find_task_by_vpid(1); + if (tsk) + get_task_struct(tsk); + rcu_read_unlock(); + if (!tsk) + return ERR_PTR(-ESRCH); + + return tsk; +} +EXPORT_SYMBOL(umh_get_init_pid); + +int umh_enter_ns(struct task_struct *tsk, struct cred *new) +{ + char path[NS_PATH_MAX]; + struct vfsmount *mnt; + const char *name; + pid_t pid; + int err = 0; + + pid = task_pid_nr(tsk); + + /* +* The user mode thread runner runs in the root init namespace +* so it will see all system pids. +*/ + mnt = task_active_pid_ns(current)->proc_mnt; + + for (name = ns_names[0]; *name; name++) { + struct file *this; + int len; + + len = snprintf(path, + NS_PATH_MAX, NS_PATH_FMT, + (unsigned long) pid, name); + if (len >= NS_PATH_MAX) { + err = -ENAMETOOLONG; + break; + } + + this = file_open_root(mnt->mnt_root, mnt, path, O_RDONLY); + if (unlikely(IS_ERR(this))) { + err = PTR_ERR(this); + break; + } + + err = setns_inode(file_inode(this), 0); + fput(this); + if (err) + break; + } + + return err; +} +EXPORT_SYMBOL(umh_enter_ns); + +static int umh_set_ns(struct subprocess_info *info, struct cred *new) +{ + struct task_struct *tsk = info->data; + + return umh_enter_ns(tsk, new); +} + +static void umh_free_ns(struct subprocess_info *info) +{ + struct task_struct *tsk = info->data; + + if (tsk) + put_task_struct(tsk); +} +#else +static int umh_set_ns(struct subprocess_info *info, struct cred *new) +{ + return 0; +} + +static void umh_free_ns(struct subprocess_info *info) +{ +} +#endif + /** * call_usermodehelper() - prepare and start a usermode application * @path: path to usermode executable @@ -599,11 +691,28 @@ int call_usermodehelper(char *path, char **argv, char **envp, int flags) { struct subprocess_info *info; gfp_t gfp_mask = (flags == UMH_NO_WAI
[RFC PATCH 7/8] nfs - cache_lib use namespace if not executing in init namespace
If the caller is running within a container then execute the usermode helper callback within the init namespace of the container. Signed-off-by: Ian Kent Cc: Benjamin Coddington Cc: Al Viro Cc: J. Bruce Fields Cc: David Howells Cc: Trond Myklebust Cc: Oleg Nesterov Cc: Eric W. Biederman Cc: Jeff Layton --- fs/nfs/cache_lib.c|6 +- fs/nfsd/nfs4recover.c |2 +- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/fs/nfs/cache_lib.c b/fs/nfs/cache_lib.c index 5f7b053..1f7f6c1 100644 --- a/fs/nfs/cache_lib.c +++ b/fs/nfs/cache_lib.c @@ -44,11 +44,15 @@ int nfs_cache_upcall(struct cache_detail *cd, char *entry_name) entry_name, NULL }; + int umh_flags = UMH_WAIT_EXEC; int ret = -EACCES; + if (cd->net != &init_net) + umh_flags |= UMH_USE_NS; + if (nfs_cache_getent_prog[0] == '\0') goto out; - ret = call_usermodehelper(argv[0], argv, envp, UMH_WAIT_EXEC); + ret = call_usermodehelper(argv[0], argv, envp, umh_flags); /* * Disable the upcall mechanism if we're getting an ENOENT or * EACCES error. The admin can re-enable it on the fly by using diff --git a/fs/nfsd/nfs4recover.c b/fs/nfsd/nfs4recover.c index b962856..0896ca7 100644 --- a/fs/nfsd/nfs4recover.c +++ b/fs/nfsd/nfs4recover.c @@ -1380,7 +1380,7 @@ nfsd4_umh_cltrack_check(struct nfs4_client *clp) } else { ret = nfsd4_umh_cltrack_upcall("check", hexid, has_session, legacy, - mm->umh_flags); + nn->umh_flags); if (ret == 0) set_bit(NFSD4_CLIENT_STABLE, &clp->cl_flags); } -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[RFC PATCH 4/8] KEYS - rename call_usermodehelper_keys() flags parameter
The wait parameter of call_usermodehelper_keys() will later be used to request exec within a namespace. So change its name to flags. Signed-off-by: Ian Kent Cc: Benjamin Coddington Cc: Al Viro Cc: J. Bruce Fields Cc: David Howells Cc: Trond Myklebust Cc: Oleg Nesterov Cc: Eric W. Biederman Cc: Jeff Layton --- security/keys/request_key.c |4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/security/keys/request_key.c b/security/keys/request_key.c index 0c7aea4..9e79bbf 100644 --- a/security/keys/request_key.c +++ b/security/keys/request_key.c @@ -73,7 +73,7 @@ static void umh_keys_cleanup(struct subprocess_info *info) * Call a usermode helper with a specific session keyring. */ static int call_usermodehelper_keys(char *path, char **argv, char **envp, - struct key *session_keyring, int wait) + struct key *session_keyring, int flags) { struct subprocess_info *info; @@ -84,7 +84,7 @@ static int call_usermodehelper_keys(char *path, char **argv, char **envp, return -ENOMEM; key_get(session_keyring); - return call_usermodehelper_exec(info, wait); + return call_usermodehelper_exec(info, flags); } /* -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[RFC PATCH 6/8] nfsd - use namespace if not executing in init namespace
If nfsd is running within a container the client tracking operations should run within the container also. Signed-off-by: Ian Kent Cc: Benjamin Coddington Cc: Al Viro Cc: J. Bruce Fields Cc: David Howells Cc: Trond Myklebust Cc: Oleg Nesterov Cc: Eric W. Biederman Cc: Jeff Layton --- fs/nfsd/netns.h |2 ++ fs/nfsd/nfs4recover.c | 48 2 files changed, 34 insertions(+), 16 deletions(-) diff --git a/fs/nfsd/netns.h b/fs/nfsd/netns.h index ea6749a..c168196 100644 --- a/fs/nfsd/netns.h +++ b/fs/nfsd/netns.h @@ -112,6 +112,8 @@ struct nfsd_net { u32 clientid_counter; struct svc_serv *nfsd_serv; + + int umh_flags; }; /* Simple check to find out if a given net was properly initialized */ diff --git a/fs/nfsd/nfs4recover.c b/fs/nfsd/nfs4recover.c index cc6a760..b962856 100644 --- a/fs/nfsd/nfs4recover.c +++ b/fs/nfsd/nfs4recover.c @@ -1184,7 +1184,9 @@ nfsd4_cltrack_grace_start(time_t grace_start) } static int -nfsd4_umh_cltrack_upcall(char *cmd, char *arg, char *env0, char *env1) +nfsd4_umh_cltrack_upcall(char *cmd, +char *arg, char *env0, char *env1, +int flags) { char *envp[3]; char *argv[4]; @@ -1209,7 +1211,7 @@ nfsd4_umh_cltrack_upcall(char *cmd, char *arg, char *env0, char *env1) argv[2] = arg; argv[3] = NULL; - ret = call_usermodehelper(argv[0], argv, envp, UMH_WAIT_PROC); + ret = call_usermodehelper(argv[0], argv, envp, flags); /* * Disable the upcall mechanism if we're getting an ENOENT or EACCES * error. The admin can re-enable it on the fly by using sysfs @@ -1252,14 +1254,13 @@ nfsd4_umh_cltrack_init(struct net *net) struct nfsd_net *nn = net_generic(net, nfsd_net_id); char *grace_start = nfsd4_cltrack_grace_start(nn->boot_time); - /* XXX: The usermode helper s not working in container yet. */ - if (net != &init_net) { - WARN(1, KERN_ERR "NFSD: attempt to initialize umh client " - "tracking in a container!\n"); - return -EINVAL; - } + nn->umh_flags = UMH_WAIT_PROC; + if (net != &init_net) + nn->umh_flags |= UMH_USE_NS; - ret = nfsd4_umh_cltrack_upcall("init", NULL, grace_start, NULL); + ret = nfsd4_umh_cltrack_upcall("init", + NULL, grace_start, NULL, + nn->umh_flags); kfree(grace_start); return ret; } @@ -1285,6 +1286,7 @@ nfsd4_umh_cltrack_create(struct nfs4_client *clp) { char *hexid, *has_session, *grace_start; struct nfsd_net *nn = net_generic(clp->net, nfsd_net_id); + int ret; /* * With v4.0 clients, there's little difference in outcome between a @@ -1312,7 +1314,10 @@ nfsd4_umh_cltrack_create(struct nfs4_client *clp) grace_start = nfsd4_cltrack_grace_start(nn->boot_time); nfsd4_cltrack_upcall_lock(clp); - if (!nfsd4_umh_cltrack_upcall("create", hexid, has_session, grace_start)) + ret = nfsd4_umh_cltrack_upcall("create", + hexid, has_session, grace_start, + nn->umh_flags); + if (!ret) set_bit(NFSD4_CLIENT_STABLE, &clp->cl_flags); nfsd4_cltrack_upcall_unlock(clp); @@ -1324,7 +1329,9 @@ nfsd4_umh_cltrack_create(struct nfs4_client *clp) static void nfsd4_umh_cltrack_remove(struct nfs4_client *clp) { + struct nfsd_net *nn = net_generic(clp->net, nfsd_net_id); char *hexid; + int ret; if (!test_bit(NFSD4_CLIENT_STABLE, &clp->cl_flags)) return; @@ -1336,9 +1343,13 @@ nfsd4_umh_cltrack_remove(struct nfs4_client *clp) } nfsd4_cltrack_upcall_lock(clp); - if (test_bit(NFSD4_CLIENT_STABLE, &clp->cl_flags) && - nfsd4_umh_cltrack_upcall("remove", hexid, NULL, NULL) == 0) - clear_bit(NFSD4_CLIENT_STABLE, &clp->cl_flags); + if (test_bit(NFSD4_CLIENT_STABLE, &clp->cl_flags)) { + ret = nfsd4_umh_cltrack_upcall("remove", + hexid, NULL, NULL, + nn->umh_flags); + if (ret == 0) + clear_bit(NFSD4_CLIENT_STABLE, &clp->cl_flags); + } nfsd4_cltrack_upcall_unlock(clp); kfree(hexid); @@ -1347,8 +1358,9 @@ nfsd4_umh_cltrack_remove(struct nfs4_client *clp) static int nfsd4_umh_cltrack_check(struct nfs4_client *clp) { - int ret; + struct nfsd_net *nn = net_generic(clp->net, nfsd_net_id); char *hexid, *has_session, *legacy; + int ret;
[RFC PATCH 1/8] nsproxy - refactor setns()
For usermode helpers to execute within a namspace a slightly different entry point to setns() that takes a namspace inode is needed. Signed-off-by: Ian Kent Cc: Benjamin Coddington Cc: Al Viro Cc: J. Bruce Fields Cc: David Howells Cc: Trond Myklebust Cc: Oleg Nesterov Cc: Eric W. Biederman Cc: Jeff Layton --- include/linux/nsproxy.h |1 + kernel/nsproxy.c| 21 ++--- 2 files changed, 15 insertions(+), 7 deletions(-) diff --git a/include/linux/nsproxy.h b/include/linux/nsproxy.h index 35fa08f..c75bf12 100644 --- a/include/linux/nsproxy.h +++ b/include/linux/nsproxy.h @@ -62,6 +62,7 @@ extern struct nsproxy init_nsproxy; * */ +int setns_inode(struct inode *inode, int nstype); int copy_namespaces(unsigned long flags, struct task_struct *tsk); void exit_task_namespaces(struct task_struct *tsk); void switch_task_namespaces(struct task_struct *tsk, struct nsproxy *new); diff --git a/kernel/nsproxy.c b/kernel/nsproxy.c index 49746c8..27cc544 100644 --- a/kernel/nsproxy.c +++ b/kernel/nsproxy.c @@ -218,20 +218,15 @@ void exit_task_namespaces(struct task_struct *p) switch_task_namespaces(p, NULL); } -SYSCALL_DEFINE2(setns, int, fd, int, nstype) +int setns_inode(struct inode *inode, int nstype) { struct task_struct *tsk = current; struct nsproxy *new_nsproxy; - struct file *file; struct ns_common *ns; int err; - file = proc_ns_fget(fd); - if (IS_ERR(file)) - return PTR_ERR(file); - err = -EINVAL; - ns = get_proc_ns(file_inode(file)); + ns = get_proc_ns(inode); if (nstype && (ns->ops->type != nstype)) goto out; @@ -248,6 +243,18 @@ SYSCALL_DEFINE2(setns, int, fd, int, nstype) } switch_task_namespaces(tsk, new_nsproxy); out: + return err; +} + +SYSCALL_DEFINE2(setns, int, fd, int, nstype) +{ + struct file *file; + int err; + + file = proc_ns_fget(fd); + if (IS_ERR(file)) + return PTR_ERR(file); + err = setns_inode(file_inode(file), nstype); fput(file); return err; } -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC PATCH 1/8] nsproxy - refactor setns()
Oops! Please ignore these, mistakenly sent. On Tue, 2015-02-03 at 15:16 +0800, Ian Kent wrote: > For usermode helpers to execute within a namspace a slightly different > entry point to setns() that takes a namspace inode is needed. > > Signed-off-by: Ian Kent > Cc: Benjamin Coddington > Cc: Al Viro > Cc: J. Bruce Fields > Cc: David Howells > Cc: Trond Myklebust > Cc: Oleg Nesterov > Cc: Eric W. Biederman > Cc: Jeff Layton > --- > include/linux/nsproxy.h |1 + > kernel/nsproxy.c| 21 ++--- > 2 files changed, 15 insertions(+), 7 deletions(-) > > diff --git a/include/linux/nsproxy.h b/include/linux/nsproxy.h > index 35fa08f..c75bf12 100644 > --- a/include/linux/nsproxy.h > +++ b/include/linux/nsproxy.h > @@ -62,6 +62,7 @@ extern struct nsproxy init_nsproxy; > * > */ > > +int setns_inode(struct inode *inode, int nstype); > int copy_namespaces(unsigned long flags, struct task_struct *tsk); > void exit_task_namespaces(struct task_struct *tsk); > void switch_task_namespaces(struct task_struct *tsk, struct nsproxy *new); > diff --git a/kernel/nsproxy.c b/kernel/nsproxy.c > index 49746c8..27cc544 100644 > --- a/kernel/nsproxy.c > +++ b/kernel/nsproxy.c > @@ -218,20 +218,15 @@ void exit_task_namespaces(struct task_struct *p) > switch_task_namespaces(p, NULL); > } > > -SYSCALL_DEFINE2(setns, int, fd, int, nstype) > +int setns_inode(struct inode *inode, int nstype) > { > struct task_struct *tsk = current; > struct nsproxy *new_nsproxy; > - struct file *file; > struct ns_common *ns; > int err; > > - file = proc_ns_fget(fd); > - if (IS_ERR(file)) > - return PTR_ERR(file); > - > err = -EINVAL; > - ns = get_proc_ns(file_inode(file)); > + ns = get_proc_ns(inode); > if (nstype && (ns->ops->type != nstype)) > goto out; > > @@ -248,6 +243,18 @@ SYSCALL_DEFINE2(setns, int, fd, int, nstype) > } > switch_task_namespaces(tsk, new_nsproxy); > out: > + return err; > +} > + > +SYSCALL_DEFINE2(setns, int, fd, int, nstype) > +{ > + struct file *file; > + int err; > + > + file = proc_ns_fget(fd); > + if (IS_ERR(file)) > + return PTR_ERR(file); > + err = setns_inode(file_inode(file), nstype); > fput(file); > return err; > } > > -- > To unsubscribe from this list: send the line "unsubscribe linux-kernel" in > the body of a message to majord...@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > Please read the FAQ at http://www.tux.org/lkml/ -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[RFC PATCH 1/8] nsproxy - refactor setns()
For usermode helpers to execute within a namspace a slightly different entry point to setns() that takes a namspace inode is needed. Signed-off-by: Ian Kent Cc: Benjamin Coddington Cc: Al Viro Cc: J. Bruce Fields Cc: David Howells Cc: Trond Myklebust Cc: Oleg Nesterov Cc: Eric W. Biederman Cc: Jeff Layton --- include/linux/nsproxy.h |1 + kernel/nsproxy.c| 21 ++--- 2 files changed, 15 insertions(+), 7 deletions(-) diff --git a/include/linux/nsproxy.h b/include/linux/nsproxy.h index 35fa08f..c75bf12 100644 --- a/include/linux/nsproxy.h +++ b/include/linux/nsproxy.h @@ -62,6 +62,7 @@ extern struct nsproxy init_nsproxy; * */ +int setns_inode(struct inode *inode, int nstype); int copy_namespaces(unsigned long flags, struct task_struct *tsk); void exit_task_namespaces(struct task_struct *tsk); void switch_task_namespaces(struct task_struct *tsk, struct nsproxy *new); diff --git a/kernel/nsproxy.c b/kernel/nsproxy.c index 49746c8..27cc544 100644 --- a/kernel/nsproxy.c +++ b/kernel/nsproxy.c @@ -218,20 +218,15 @@ void exit_task_namespaces(struct task_struct *p) switch_task_namespaces(p, NULL); } -SYSCALL_DEFINE2(setns, int, fd, int, nstype) +int setns_inode(struct inode *inode, int nstype) { struct task_struct *tsk = current; struct nsproxy *new_nsproxy; - struct file *file; struct ns_common *ns; int err; - file = proc_ns_fget(fd); - if (IS_ERR(file)) - return PTR_ERR(file); - err = -EINVAL; - ns = get_proc_ns(file_inode(file)); + ns = get_proc_ns(inode); if (nstype && (ns->ops->type != nstype)) goto out; @@ -248,6 +243,18 @@ SYSCALL_DEFINE2(setns, int, fd, int, nstype) } switch_task_namespaces(tsk, new_nsproxy); out: + return err; +} + +SYSCALL_DEFINE2(setns, int, fd, int, nstype) +{ + struct file *file; + int err; + + file = proc_ns_fget(fd); + if (IS_ERR(file)) + return PTR_ERR(file); + err = setns_inode(file_inode(file), nstype); fput(file); return err; } -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[RFC PATCH 2/8] kmod - rename call_usermodehelper() flags parameter
The wait parameter of call_usermodehelper() is not quite a parameter that describes the wait behaviour alone and will later be used to request exec within a namespace. So change its name to flags. Signed-off-by: Ian Kent Cc: Benjamin Coddington Cc: Al Viro Cc: J. Bruce Fields Cc: David Howells Cc: Trond Myklebust Cc: Oleg Nesterov Cc: Eric W. Biederman Cc: Jeff Layton --- include/linux/kmod.h |4 ++-- kernel/kmod.c| 16 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/include/linux/kmod.h b/include/linux/kmod.h index 0555cc6..15bdeed 100644 --- a/include/linux/kmod.h +++ b/include/linux/kmod.h @@ -67,7 +67,7 @@ struct subprocess_info { }; extern int -call_usermodehelper(char *path, char **argv, char **envp, int wait); +call_usermodehelper(char *path, char **argv, char **envp, int flags); extern struct subprocess_info * call_usermodehelper_setup(char *path, char **argv, char **envp, gfp_t gfp_mask, @@ -75,7 +75,7 @@ call_usermodehelper_setup(char *path, char **argv, char **envp, gfp_t gfp_mask, void (*cleanup)(struct subprocess_info *), void *data); extern int -call_usermodehelper_exec(struct subprocess_info *info, int wait); +call_usermodehelper_exec(struct subprocess_info *info, int flags); extern struct ctl_table usermodehelper_table[]; diff --git a/kernel/kmod.c b/kernel/kmod.c index 2777f40..14c0188 100644 --- a/kernel/kmod.c +++ b/kernel/kmod.c @@ -534,7 +534,7 @@ EXPORT_SYMBOL(call_usermodehelper_setup); * asynchronously if wait is not set, and runs as a child of keventd. * (ie. it runs with full root capabilities). */ -int call_usermodehelper_exec(struct subprocess_info *sub_info, int wait) +int call_usermodehelper_exec(struct subprocess_info *sub_info, int flags) { DECLARE_COMPLETION_ONSTACK(done); int retval = 0; @@ -553,14 +553,14 @@ int call_usermodehelper_exec(struct subprocess_info *sub_info, int wait) * This makes it possible to use umh_complete to free * the data structure in case of UMH_NO_WAIT. */ - sub_info->complete = (wait == UMH_NO_WAIT) ? NULL : &done; - sub_info->wait = wait; + sub_info->complete = (flags == UMH_NO_WAIT) ? NULL : &done; + sub_info->wait = flags; queue_work(khelper_wq, &sub_info->work); - if (wait == UMH_NO_WAIT)/* task has freed sub_info */ + if (flags == UMH_NO_WAIT) /* task has freed sub_info */ goto unlock; - if (wait & UMH_KILLABLE) { + if (flags & UMH_KILLABLE) { retval = wait_for_completion_killable(&done); if (!retval) goto wait_done; @@ -595,17 +595,17 @@ EXPORT_SYMBOL(call_usermodehelper_exec); * This function is the equivalent to use call_usermodehelper_setup() and * call_usermodehelper_exec(). */ -int call_usermodehelper(char *path, char **argv, char **envp, int wait) +int call_usermodehelper(char *path, char **argv, char **envp, int flags) { struct subprocess_info *info; - gfp_t gfp_mask = (wait == UMH_NO_WAIT) ? GFP_ATOMIC : GFP_KERNEL; + gfp_t gfp_mask = (flags == UMH_NO_WAIT) ? GFP_ATOMIC : GFP_KERNEL; info = call_usermodehelper_setup(path, argv, envp, gfp_mask, NULL, NULL, NULL); if (info == NULL) return -ENOMEM; - return call_usermodehelper_exec(info, wait); + return call_usermodehelper_exec(info, flags); } EXPORT_SYMBOL(call_usermodehelper); -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC PATCH 0/5] Second attempt at contained helper execution
On Wed, 2015-01-21 at 09:38 -0500, J. Bruce Fields wrote: > On Wed, Jan 21, 2015 at 03:05:25PM +0800, Ian Kent wrote: > > On Fri, 2015-01-16 at 10:25 -0500, J. Bruce Fields wrote: > > > On Fri, Jan 16, 2015 at 09:01:13AM +0800, Ian Kent wrote: > > > > On Thu, 2015-01-15 at 11:27 -0500, J. Bruce Fields wrote: > > > > > On Thu, Jan 15, 2015 at 08:26:12AM +0800, Ian Kent wrote: > > > > > > On Wed, 2015-01-14 at 17:10 -0500, J. Bruce Fields wrote: > > > > > > > > On Wed, Jan 14, 2015 at 05:32:22PM +0800, Ian Kent wrote: > > > > > > > > > There are other difficulties to tackle as well, such as how > > > > > > > > > to decide > > > > > > > > > if contained helper execution is needed. For example, if a > > > > > > > > > mount has > > > > > > > > > been propagated to a container or bound into the container > > > > > > > > > tree (such > > > > > > > > > as with the --volume option of "docker run") the root init > > > > > > > > > namespace > > > > > > > > > may need to be used and not the container namespace. > > > > > > > > > > > > > > I think you have to go through each of the existing upcall > > > > > > > examples and > > > > > > > decide what's needed for each. > > > > > > > > > > > > > > At least for the nfsv4 idmapper I would've thought the namespace > > > > > > > the > > > > > > > mount was done in would be the right choice, hence my previous > > > > > > > question. > > > > > > > > > > > > Probably but you don't necessarily know what namespace the mount was > > > > > > done in. It may have been propagated from another namespace or > > > > > > (although > > > > > > I don't think it works yet) bound from another container using the > > > > > > volumes-from docker option. > > > > > > > > > > Name-id mappings should be associated with the superblock, I guess--so > > > > > don't you store a pointer to the right thing there? > > > > > > > > Quite possibly but my original point was, without an acceptable > > > > mechanism to execute the helper we can't know what might need to be done > > > > to use it. > > > > > > At least for me it would be easier to review if it came with at least > > > one example user. > > > > Haven't seen any negative responses but perhaps people are still away > > over Xmas. > > > > In the mean time it's probably a good idea to add some use cases to the > > series in case the approach is OK. > > > > I'll have a look at the nfsd code and see if I can spot the places. > > On the nfsd side it's just the one call_usermodehelper in > fs/nfsd/nfs4recover.c. The tricky part is figuring out where the > namespace information should come from. I had a look at the nfsd code but haven't looked at the nfsdcltrack to see what it does and if it expects anything that wouldn't be available. There's also the assumption that the external application is present within the container filesystem at the same location. The whole idea of the current approach is that the namespace information comes from the init process of the container it's executing in. I believe that if nfsd is running in a container it should be able to function entirely within the container, except for the callback issue. I see there's a check in the callback init function to see if the net namespace in use is the root net namespace. It looks like that check would be enough to determine that container execution is needed. Assuming that the various locations all contain the same struct net, such as is stored in (struct nfs4_client) clp, then it's probably enough to change nfsd4_umh_cltrack_upcall() to also take net as a parameter. Then the same check (which would be removed) used in the init function could be used to determine if the UMH_USE_NS flag needs to be passed to call_usermodehelper(). Passing the UMH_USE_NS flag to call_usermodehelper() will cause the helper to be executed within the init namespace (including the net namespace) of the container. If that is what's needed then it might be sensible to change nfsd4_umh_cltrack_upcall() to take a structure containing parameters to keep it clean. I could create patches to demonstrate the procedure but we probably should keep that discussion separate from this one for the moment. Ian -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC PATCH 0/5] Second attempt at contained helper execution
On Fri, 2015-01-16 at 10:25 -0500, J. Bruce Fields wrote: > On Fri, Jan 16, 2015 at 09:01:13AM +0800, Ian Kent wrote: > > On Thu, 2015-01-15 at 11:27 -0500, J. Bruce Fields wrote: > > > On Thu, Jan 15, 2015 at 08:26:12AM +0800, Ian Kent wrote: > > > > On Wed, 2015-01-14 at 17:10 -0500, J. Bruce Fields wrote: > > > > > > On Wed, Jan 14, 2015 at 05:32:22PM +0800, Ian Kent wrote: > > > > > > > There are other difficulties to tackle as well, such as how to > > > > > > > decide > > > > > > > if contained helper execution is needed. For example, if a mount > > > > > > > has > > > > > > > been propagated to a container or bound into the container tree > > > > > > > (such > > > > > > > as with the --volume option of "docker run") the root init > > > > > > > namespace > > > > > > > may need to be used and not the container namespace. > > > > > > > > > > I think you have to go through each of the existing upcall examples > > > > > and > > > > > decide what's needed for each. > > > > > > > > > > At least for the nfsv4 idmapper I would've thought the namespace the > > > > > mount was done in would be the right choice, hence my previous > > > > > question. > > > > > > > > Probably but you don't necessarily know what namespace the mount was > > > > done in. It may have been propagated from another namespace or (although > > > > I don't think it works yet) bound from another container using the > > > > volumes-from docker option. > > > > > > Name-id mappings should be associated with the superblock, I guess--so > > > don't you store a pointer to the right thing there? > > > > Quite possibly but my original point was, without an acceptable > > mechanism to execute the helper we can't know what might need to be done > > to use it. > > At least for me it would be easier to review if it came with at least > one example user. Haven't seen any negative responses but perhaps people are still away over Xmas. In the mean time it's probably a good idea to add some use cases to the series in case the approach is OK. I'll have a look at the nfsd code and see if I can spot the places. Ian -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC PATCH 3/5] kmod - teach call_usermodehelper() to use a namespace
On Thu, 2015-01-15 at 11:45 -0500, Jeff Layton wrote: > On Wed, 14 Jan 2015 17:32:43 +0800 > Ian Kent wrote: > > > The call_usermodehelper() function executes all binaries in the > > global "init" root context. This doesn't allow a binary to be run > > within a namespace (eg. the namespace of a container). > > > > Both containerized NFS client and NFS server need the ability to > > execute a binary in a container's context. To do this use the init > > process of the callers environment is used to setup the namespaces > > in the same way the root init process is used otherwise. > > > > Signed-off-by: Ian Kent > > Cc: Benjamin Coddington > > Cc: Al Viro > > Cc: J. Bruce Fields > > Cc: David Howells > > Cc: Trond Myklebust > > Cc: Oleg Nesterov > > Cc: Eric W. Biederman > > Cc: Jeff Layton > > --- > > include/linux/kmod.h | 17 +++ > > kernel/kmod.c| 119 > > +- > > 2 files changed, 133 insertions(+), 3 deletions(-) > > > > diff --git a/include/linux/kmod.h b/include/linux/kmod.h > > index 15bdeed..487e68e 100644 > > --- a/include/linux/kmod.h > > +++ b/include/linux/kmod.h > > @@ -52,6 +52,7 @@ struct file; > > #define UMH_WAIT_EXEC 1 /* wait for the exec, but not the > > process */ > > #define UMH_WAIT_PROC 2 /* wait for the process to complete */ > > #define UMH_KILLABLE 4 /* wait for EXEC/PROC killable */ > > +#define UMH_USE_NS 8 /* exec using caller's init namespace */ > > > > struct subprocess_info { > > struct work_struct work; > > @@ -69,6 +70,22 @@ struct subprocess_info { > > extern int > > call_usermodehelper(char *path, char **argv, char **envp, int flags); > > > > +#if !defined(CONFIG_PROC_FS) || !defined(CONFIG_NAMESPACES) > > +inline int umh_get_init_pid(pid_t *pid) > > +{ > > + *pid = 0; > > + return 0; > > +} > > + > > +inline int umh_enter_ns(pid_t pid, struct cred *new) > > +{ > > + return -ENOTSUP; > > +} > > +#else > > +int umh_get_init_pid(pid_t *pid); > > +int umh_enter_ns(pid_t pid, struct cred *new); > > +#endif > > + > > extern struct subprocess_info * > > call_usermodehelper_setup(char *path, char **argv, char **envp, gfp_t > > gfp_mask, > > int (*init)(struct subprocess_info *info, struct cred > > *new), > > diff --git a/kernel/kmod.c b/kernel/kmod.c > > index 14c0188..2179e58 100644 > > --- a/kernel/kmod.c > > +++ b/kernel/kmod.c > > @@ -582,6 +582,97 @@ unlock: > > } > > EXPORT_SYMBOL(call_usermodehelper_exec); > > > > +#if defined(CONFIG_PROC_FS) && defined(CONFIG_NAMESPACES) > > +#define NS_PATH_MAX35 > > +#define NS_PATH_FMT"%lu/ns/%s" > > + > > +/* Note namespace name order is significant */ > > +static const char *ns_names[] = { "user", "ipc", "uts", "net", "pid", > > "mnt", NULL }; > > + > > +int umh_get_init_pid(pid_t *pid) > > +{ > > + struct task_struct *tsk; > > + pid_t init_pid; > > + > > + rcu_read_lock(); > > + tsk = find_task_by_vpid(1); > > + if (tsk) > > + get_task_struct(tsk); > > + rcu_read_unlock(); > > + if (!tsk) > > + return -ESRCH; > > + init_pid = task_pid_nr(tsk); > > + put_task_struct(tsk); > > + > > + *pid = init_pid; > > + > > + return 0; > > +} > > +EXPORT_SYMBOL(umh_get_init_pid); > > + > > nit: pid_t == int, so you could just make this function return the > pid or a negative error code instead of dealing with a return argument. True, but it worries me that pid_t might one day become an unsigned int or long and something like that would be a hidden problem. > > > +int umh_enter_ns(pid_t pid, struct cred *new) > > +{ > > + char path[NS_PATH_MAX]; > > + struct vfsmount *mnt; > > + const char *name; > > + int err = 0; > > + > > + /* > > +* The user mode thread runner runs in the root init namespace > > +* so it will see all system pids. > > +*/ > > + mnt = task_active_pid_ns(current)->proc_mnt; > > + > > + for (name = ns_names[0]; *name; name++) { > > + struct file *this; > > + int len; > > + > > + len = snprintf(path, > > +
Re: [RFC PATCH 0/5] Second attempt at contained helper execution
On Thu, 2015-01-15 at 11:27 -0500, J. Bruce Fields wrote: > On Thu, Jan 15, 2015 at 08:26:12AM +0800, Ian Kent wrote: > > On Wed, 2015-01-14 at 17:10 -0500, J. Bruce Fields wrote: > > > > On Wed, Jan 14, 2015 at 05:32:22PM +0800, Ian Kent wrote: > > > > > There are other difficulties to tackle as well, such as how to decide > > > > > if contained helper execution is needed. For example, if a mount has > > > > > been propagated to a container or bound into the container tree (such > > > > > as with the --volume option of "docker run") the root init namespace > > > > > may need to be used and not the container namespace. > > > > > > I think you have to go through each of the existing upcall examples and > > > decide what's needed for each. > > > > > > At least for the nfsv4 idmapper I would've thought the namespace the > > > mount was done in would be the right choice, hence my previous question. > > > > Probably but you don't necessarily know what namespace the mount was > > done in. It may have been propagated from another namespace or (although > > I don't think it works yet) bound from another container using the > > volumes-from docker option. > > Name-id mappings should be associated with the superblock, I guess--so > don't you store a pointer to the right thing there? Quite possibly but my original point was, without an acceptable mechanism to execute the helper we can't know what might need to be done to use it. > > --b. > > > > > At least I believe that's a problem and I agree that, once a suitable > > method of running helpers is found each case will need to be looked at. > > > > Ian > > > > -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC PATCH 0/5] Second attempt at contained helper execution
On Wed, 2015-01-14 at 17:10 -0500, J. Bruce Fields wrote: > > On Wed, Jan 14, 2015 at 05:32:22PM +0800, Ian Kent wrote: > > > There are other difficulties to tackle as well, such as how to decide > > > if contained helper execution is needed. For example, if a mount has > > > been propagated to a container or bound into the container tree (such > > > as with the --volume option of "docker run") the root init namespace > > > may need to be used and not the container namespace. > > I think you have to go through each of the existing upcall examples and > decide what's needed for each. > > At least for the nfsv4 idmapper I would've thought the namespace the > mount was done in would be the right choice, hence my previous question. Probably but you don't necessarily know what namespace the mount was done in. It may have been propagated from another namespace or (although I don't think it works yet) bound from another container using the volumes-from docker option. At least I believe that's a problem and I agree that, once a suitable method of running helpers is found each case will need to be looked at. Ian -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[RFC PATCH 3/5] kmod - teach call_usermodehelper() to use a namespace
The call_usermodehelper() function executes all binaries in the global "init" root context. This doesn't allow a binary to be run within a namespace (eg. the namespace of a container). Both containerized NFS client and NFS server need the ability to execute a binary in a container's context. To do this use the init process of the callers environment is used to setup the namespaces in the same way the root init process is used otherwise. Signed-off-by: Ian Kent Cc: Benjamin Coddington Cc: Al Viro Cc: J. Bruce Fields Cc: David Howells Cc: Trond Myklebust Cc: Oleg Nesterov Cc: Eric W. Biederman Cc: Jeff Layton --- include/linux/kmod.h | 17 +++ kernel/kmod.c| 119 +- 2 files changed, 133 insertions(+), 3 deletions(-) diff --git a/include/linux/kmod.h b/include/linux/kmod.h index 15bdeed..487e68e 100644 --- a/include/linux/kmod.h +++ b/include/linux/kmod.h @@ -52,6 +52,7 @@ struct file; #define UMH_WAIT_EXEC 1 /* wait for the exec, but not the process */ #define UMH_WAIT_PROC 2 /* wait for the process to complete */ #define UMH_KILLABLE 4 /* wait for EXEC/PROC killable */ +#define UMH_USE_NS 8 /* exec using caller's init namespace */ struct subprocess_info { struct work_struct work; @@ -69,6 +70,22 @@ struct subprocess_info { extern int call_usermodehelper(char *path, char **argv, char **envp, int flags); +#if !defined(CONFIG_PROC_FS) || !defined(CONFIG_NAMESPACES) +inline int umh_get_init_pid(pid_t *pid) +{ + *pid = 0; + return 0; +} + +inline int umh_enter_ns(pid_t pid, struct cred *new) +{ + return -ENOTSUP; +} +#else +int umh_get_init_pid(pid_t *pid); +int umh_enter_ns(pid_t pid, struct cred *new); +#endif + extern struct subprocess_info * call_usermodehelper_setup(char *path, char **argv, char **envp, gfp_t gfp_mask, int (*init)(struct subprocess_info *info, struct cred *new), diff --git a/kernel/kmod.c b/kernel/kmod.c index 14c0188..2179e58 100644 --- a/kernel/kmod.c +++ b/kernel/kmod.c @@ -582,6 +582,97 @@ unlock: } EXPORT_SYMBOL(call_usermodehelper_exec); +#if defined(CONFIG_PROC_FS) && defined(CONFIG_NAMESPACES) +#define NS_PATH_MAX35 +#define NS_PATH_FMT"%lu/ns/%s" + +/* Note namespace name order is significant */ +static const char *ns_names[] = { "user", "ipc", "uts", "net", "pid", "mnt", NULL }; + +int umh_get_init_pid(pid_t *pid) +{ + struct task_struct *tsk; + pid_t init_pid; + + rcu_read_lock(); + tsk = find_task_by_vpid(1); + if (tsk) + get_task_struct(tsk); + rcu_read_unlock(); + if (!tsk) + return -ESRCH; + init_pid = task_pid_nr(tsk); + put_task_struct(tsk); + + *pid = init_pid; + + return 0; +} +EXPORT_SYMBOL(umh_get_init_pid); + +int umh_enter_ns(pid_t pid, struct cred *new) +{ + char path[NS_PATH_MAX]; + struct vfsmount *mnt; + const char *name; + int err = 0; + + /* +* The user mode thread runner runs in the root init namespace +* so it will see all system pids. +*/ + mnt = task_active_pid_ns(current)->proc_mnt; + + for (name = ns_names[0]; *name; name++) { + struct file *this; + int len; + + len = snprintf(path, + NS_PATH_MAX, NS_PATH_FMT, + (unsigned long) pid, name); + if (len >= NS_PATH_MAX) { + err = -ENAMETOOLONG; + break; + } + + this = file_open_root(mnt->mnt_root, mnt, path, O_RDONLY); + if (unlikely(IS_ERR(this))) { + err = PTR_ERR(this); + break; + } + + err = setns_inode(file_inode(this), 0); + fput(this); + if (err) + break; + } + + return err; +} +EXPORT_SYMBOL(umh_enter_ns); + +static int umh_set_ns(struct subprocess_info *info, struct cred *new) +{ + pid_t *init_pid = (pid_t *) info->data; + + return umh_enter_ns(*init_pid, new); +} + +static void umh_free_ns(struct subprocess_info *info) +{ + kfree(info->data); +} +#else +static int umh_set_ns(struct subprocess_info *info, struct cred *new) +{ + return 0; +} + +static void umh_free_ns(struct subprocess_info *info) +{ +} +#endif + /** * call_usermodehelper() - prepare and start a usermode application * @path: path to usermode executable @@ -599,11 +690,33 @@ int call_usermodehelper(char *path, char **argv, char **envp, int flags) { struct subprocess_info *info; gfp_t gfp_mask = (flags == UMH_NO_WAIT) ? GFP_ATOMIC : GFP_KERNEL; + unsigned int use_ns = flags & UMH_USE_NS; +
[RFC PATCH 2/5] kmod - rename call_usermodehelper() flags parameter
The wait parameter of call_usermodehelper() is not quite a parameter that describes the wait behaviour alone and will later be used to request exec within a namespace. So change its name to flags. Signed-off-by: Ian Kent Cc: Benjamin Coddington Cc: Al Viro Cc: J. Bruce Fields Cc: David Howells Cc: Trond Myklebust Cc: Oleg Nesterov Cc: Eric W. Biederman Cc: Jeff Layton --- include/linux/kmod.h |4 ++-- kernel/kmod.c| 16 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/include/linux/kmod.h b/include/linux/kmod.h index 0555cc6..15bdeed 100644 --- a/include/linux/kmod.h +++ b/include/linux/kmod.h @@ -67,7 +67,7 @@ struct subprocess_info { }; extern int -call_usermodehelper(char *path, char **argv, char **envp, int wait); +call_usermodehelper(char *path, char **argv, char **envp, int flags); extern struct subprocess_info * call_usermodehelper_setup(char *path, char **argv, char **envp, gfp_t gfp_mask, @@ -75,7 +75,7 @@ call_usermodehelper_setup(char *path, char **argv, char **envp, gfp_t gfp_mask, void (*cleanup)(struct subprocess_info *), void *data); extern int -call_usermodehelper_exec(struct subprocess_info *info, int wait); +call_usermodehelper_exec(struct subprocess_info *info, int flags); extern struct ctl_table usermodehelper_table[]; diff --git a/kernel/kmod.c b/kernel/kmod.c index 2777f40..14c0188 100644 --- a/kernel/kmod.c +++ b/kernel/kmod.c @@ -534,7 +534,7 @@ EXPORT_SYMBOL(call_usermodehelper_setup); * asynchronously if wait is not set, and runs as a child of keventd. * (ie. it runs with full root capabilities). */ -int call_usermodehelper_exec(struct subprocess_info *sub_info, int wait) +int call_usermodehelper_exec(struct subprocess_info *sub_info, int flags) { DECLARE_COMPLETION_ONSTACK(done); int retval = 0; @@ -553,14 +553,14 @@ int call_usermodehelper_exec(struct subprocess_info *sub_info, int wait) * This makes it possible to use umh_complete to free * the data structure in case of UMH_NO_WAIT. */ - sub_info->complete = (wait == UMH_NO_WAIT) ? NULL : &done; - sub_info->wait = wait; + sub_info->complete = (flags == UMH_NO_WAIT) ? NULL : &done; + sub_info->wait = flags; queue_work(khelper_wq, &sub_info->work); - if (wait == UMH_NO_WAIT)/* task has freed sub_info */ + if (flags == UMH_NO_WAIT) /* task has freed sub_info */ goto unlock; - if (wait & UMH_KILLABLE) { + if (flags & UMH_KILLABLE) { retval = wait_for_completion_killable(&done); if (!retval) goto wait_done; @@ -595,17 +595,17 @@ EXPORT_SYMBOL(call_usermodehelper_exec); * This function is the equivalent to use call_usermodehelper_setup() and * call_usermodehelper_exec(). */ -int call_usermodehelper(char *path, char **argv, char **envp, int wait) +int call_usermodehelper(char *path, char **argv, char **envp, int flags) { struct subprocess_info *info; - gfp_t gfp_mask = (wait == UMH_NO_WAIT) ? GFP_ATOMIC : GFP_KERNEL; + gfp_t gfp_mask = (flags == UMH_NO_WAIT) ? GFP_ATOMIC : GFP_KERNEL; info = call_usermodehelper_setup(path, argv, envp, gfp_mask, NULL, NULL, NULL); if (info == NULL) return -ENOMEM; - return call_usermodehelper_exec(info, wait); + return call_usermodehelper_exec(info, flags); } EXPORT_SYMBOL(call_usermodehelper); -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[RFC PATCH 1/5] nsproxy - refactor setns()
For usermode helpers to execute within a namspace a slightly different entry point to setns() that takes a namspace inode is needed. Signed-off-by: Ian Kent Cc: Benjamin Coddington Cc: Al Viro Cc: J. Bruce Fields Cc: David Howells Cc: Trond Myklebust Cc: Oleg Nesterov Cc: Eric W. Biederman Cc: Jeff Layton --- include/linux/nsproxy.h |1 + kernel/nsproxy.c| 21 ++--- 2 files changed, 15 insertions(+), 7 deletions(-) diff --git a/include/linux/nsproxy.h b/include/linux/nsproxy.h index 35fa08f..c75bf12 100644 --- a/include/linux/nsproxy.h +++ b/include/linux/nsproxy.h @@ -62,6 +62,7 @@ extern struct nsproxy init_nsproxy; * */ +int setns_inode(struct inode *inode, int nstype); int copy_namespaces(unsigned long flags, struct task_struct *tsk); void exit_task_namespaces(struct task_struct *tsk); void switch_task_namespaces(struct task_struct *tsk, struct nsproxy *new); diff --git a/kernel/nsproxy.c b/kernel/nsproxy.c index 49746c8..27cc544 100644 --- a/kernel/nsproxy.c +++ b/kernel/nsproxy.c @@ -218,20 +218,15 @@ void exit_task_namespaces(struct task_struct *p) switch_task_namespaces(p, NULL); } -SYSCALL_DEFINE2(setns, int, fd, int, nstype) +int setns_inode(struct inode *inode, int nstype) { struct task_struct *tsk = current; struct nsproxy *new_nsproxy; - struct file *file; struct ns_common *ns; int err; - file = proc_ns_fget(fd); - if (IS_ERR(file)) - return PTR_ERR(file); - err = -EINVAL; - ns = get_proc_ns(file_inode(file)); + ns = get_proc_ns(inode); if (nstype && (ns->ops->type != nstype)) goto out; @@ -248,6 +243,18 @@ SYSCALL_DEFINE2(setns, int, fd, int, nstype) } switch_task_namespaces(tsk, new_nsproxy); out: + return err; +} + +SYSCALL_DEFINE2(setns, int, fd, int, nstype) +{ + struct file *file; + int err; + + file = proc_ns_fget(fd); + if (IS_ERR(file)) + return PTR_ERR(file); + err = setns_inode(file_inode(file), nstype); fput(file); return err; } -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[RFC PATCH 4/5] KEYS - rename call_usermodehelper_keys() flags parameter
The wait parameter of call_usermodehelper_keys() will later be used to request exec within a namespace. So change its name to flags. Signed-off-by: Ian Kent Cc: Benjamin Coddington Cc: Al Viro Cc: J. Bruce Fields Cc: David Howells Cc: Trond Myklebust Cc: Oleg Nesterov Cc: Eric W. Biederman Cc: Jeff Layton --- security/keys/request_key.c |4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/security/keys/request_key.c b/security/keys/request_key.c index 0c7aea4..9e79bbf 100644 --- a/security/keys/request_key.c +++ b/security/keys/request_key.c @@ -73,7 +73,7 @@ static void umh_keys_cleanup(struct subprocess_info *info) * Call a usermode helper with a specific session keyring. */ static int call_usermodehelper_keys(char *path, char **argv, char **envp, - struct key *session_keyring, int wait) + struct key *session_keyring, int flags) { struct subprocess_info *info; @@ -84,7 +84,7 @@ static int call_usermodehelper_keys(char *path, char **argv, char **envp, return -ENOMEM; key_get(session_keyring); - return call_usermodehelper_exec(info, wait); + return call_usermodehelper_exec(info, flags); } /* -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[RFC PATCH 5/5] KEYS: exec request-key within the requesting task's init namespace
Containerized request key helper callbacks need the ability to execute a binary in a container's context. To do this calling an in kernel equivalent of setns(2) should be sufficient since the user mode helper execution kernel thread ultimately calls do_execve(). Signed-off-by: Ian Kent Cc: Benjamin Coddington Cc: Al Viro Cc: J. Bruce Fields Cc: David Howells Cc: Trond Myklebust Cc: Oleg Nesterov Cc: Eric W. Biederman Cc: Jeff Layton --- security/keys/request_key.c | 47 ++- 1 file changed, 41 insertions(+), 6 deletions(-) diff --git a/security/keys/request_key.c b/security/keys/request_key.c index 9e79bbf..d986ef3 100644 --- a/security/keys/request_key.c +++ b/security/keys/request_key.c @@ -17,6 +17,8 @@ #include #include #include +#include +#include #include "internal.h" #define key_negative_timeout 60 /* default timeout on a negative key's existence */ @@ -46,6 +48,11 @@ void complete_request_key(struct key_construction *cons, int error) } EXPORT_SYMBOL(complete_request_key); +struct request_key_info { + struct key *keyring; + pid_t init_pid; +}; + /* * Initialise a usermode helper that is going to have a specific session * keyring. @@ -55,9 +62,18 @@ EXPORT_SYMBOL(complete_request_key); */ static int umh_keys_init(struct subprocess_info *info, struct cred *cred) { - struct key *keyring = info->data; + struct request_key_info *rki = info->data; + pid_t init_pid = rki->init_pid; + + if (init_pid) { + int err; + + err = umh_enter_ns(init_pid, cred); + if (err) + return err; + } - return install_session_keyring_to_cred(cred, keyring); + return install_session_keyring_to_cred(cred, rki->keyring); } /* @@ -65,8 +81,9 @@ static int umh_keys_init(struct subprocess_info *info, struct cred *cred) */ static void umh_keys_cleanup(struct subprocess_info *info) { - struct key *keyring = info->data; - key_put(keyring); + struct request_key_info *rki = info->data; + key_put(rki->keyring); + kfree(rki); } /* @@ -76,12 +93,30 @@ static int call_usermodehelper_keys(char *path, char **argv, char **envp, struct key *session_keyring, int flags) { struct subprocess_info *info; + struct request_key_info *rki; + unsigned int use_ns = flags & UMH_USE_NS; + pid_t init_pid = 0; + + rki = kmalloc(sizeof(*rki), GFP_KERNEL); + if (!rki) + return -ENOMEM; + + if (use_ns) { + int ret = umh_get_init_pid(&init_pid); + if (ret) + return ret; + } + + rki->keyring = session_keyring; + rki->init_pid = init_pid; info = call_usermodehelper_setup(path, argv, envp, GFP_KERNEL, umh_keys_init, umh_keys_cleanup, - session_keyring); - if (!info) + rki); + if (!info) { + kfree(rki); return -ENOMEM; + } key_get(session_keyring); return call_usermodehelper_exec(info, flags); -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[RFC PATCH 0/5] Second attempt at contained helper execution
This series is a further attempt to find how (or even an acceptable way) to execute a usermode helper in a contained environment. Being an attempt to find how to do this no testing has been done and won't be until a suitable approach can be agreed on, if at all. >From previous discussion seperation between the caller and the execution environment is required for security reasons. It was suggested that a thread be created for each mount and be used as the basis for the execution environment. There are a number of problems with this, not the least of which is scaling to a large numbers of mounts, and there may not be a mount corresponding the the needed callback which amounts to creating the process from the context of the caller which we don't want to do. But now, when a usermode helper is executed the root init namespace is used and has proven to be adequate. So perhaps it will also be adequate to use the same approach for contained execution by using the container init namespace as the basis for the execution. That's essentially all this series attempts to do. There are other difficulties to tackle as well, such as how to decide if contained helper execution is needed. For example, if a mount has been propagated to a container or bound into the container tree (such as with the --volume option of "docker run") the root init namespace may need to be used and not the container namespace. There's also the rather resource heavy method that is used here to enter the target namespace which probably needs work but is out of scope for this series if in fact this approach is even acceptable. Comments please? --- Ian Kent (5): nsproxy - refactor setns() kmod - rename call_usermodehelper() flags parameter kmod - teach call_usermodehelper() to use a namespace KEYS - rename call_usermodehelper_keys() flags parameter KEYS: exec request-key within the requesting task's init namespace include/linux/kmod.h| 21 ++- include/linux/nsproxy.h |1 kernel/kmod.c | 135 +++ kernel/nsproxy.c| 21 --- security/keys/request_key.c | 51 ++-- 5 files changed, 201 insertions(+), 28 deletions(-) -- Ian -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC PATCH 3/4] kmod - add call_usermodehelper_ns() helper
On Wed, 2014-12-03 at 10:49 -0600, Eric W. Biederman wrote: > Ian Kent writes: > > > On Mon, 2014-12-01 at 16:56 -0500, Benjamin Coddington wrote: > >> n Tue, 25 Nov 2014, Eric W. Biederman wrote: > >> Hi, > >> > >> > Ian Kent writes: > >> > > >> > > On Tue, 2014-11-25 at 17:19 -0600, Eric W. Biederman wrote: > >> > >> Ian Kent writes: > >> > >> > >> > >> > On Tue, 2014-11-25 at 16:23 -0600, Eric W. Biederman wrote: > >> > >> >> Oleg Nesterov writes: > >> > >> >> > >> > >> >> > On 11/25, Oleg Nesterov wrote: > >> > >> >> >> > >> > >> >> >> Let me first apologize, I didn't actually read this series yet. > >> > >> >> >> > >> > >> >> >> But I have to admit that so far I do not like this approach... > >> > >> >> >> probably I am biased. > >> > >> >> > > >> > >> >> > Yes. > >> > >> >> > > >> > >> >> > And I have another concern... this is mostly a feeling, I can be > >> > >> >> > easily wrong but: > >> > >> >> > > >> > >> >> >> On 11/25, Ian Kent wrote: > >> > >> >> >> > > >> > >> >> >> > +static int umh_set_ns(struct subprocess_info *info, struct > >> > >> >> >> > cred *new) > >> > >> >> >> > +{ > >> > >> >> >> > + struct nsproxy *ns = info->data; > >> > >> >> >> > + > >> > >> >> >> > + mntns_setfs(ns->mnt_ns); > >> > >> >> >> > >> > >> >> >> Firstly, it is not clear to me if we should use the caller's > >> > >> >> >> ->mnt_ns. > >> > >> >> >> Let me remind about the coredump. The dumping task can cloned > >> > >> >> >> with > >> > >> >> >> CLONE_NEWNS or it cam do unshare(NEWNS)... but OK, I do not > >> > >> >> >> understand > >> > >> >> >> this enough. > >> > >> >> > > >> > >> >> > And otoh. If we actually want to use the caller's > >> > >> >> > mnt_ns/namespaces we > >> > >> >> > could simply fork/reparent a child which will do execve ? > >> > >> >> > >> > >> >> That would certainly be a better approach, and roughly equivalent > >> > >> >> to > >> > >> >> what exists here. That would even ensure we remain in the proper > >> > >> >> cgroups, and lsm context. > >> > >> >> > >> > >> >> The practical problem with the approach presented here is that I > >> > >> >> can > >> > >> >> hijack any user mode helper I wish, and make it run in any > >> > >> >> executable I > >> > >> >> wish as the global root user. > >> > >> >> > >> > >> >> Ian if we were to merge this I believe you would win the award for > >> > >> >> easiest path to a root shell. > >> > >> > > >> > >> > LOL, OK, so there's a problem with this. > >> > >> > > >> > >> > But, how should a user mode helper execute within a namespace (or > >> > >> > more > >> > >> > specifically within a container)? > >> > >> > > >> > >> > Suppose a user mode helper program scans through the pid list and > >> > >> > somehow picks the correct process pid and then does an > >> > >> > open()/setns()/execve(). > >> > >> > > >> > >> > Does that then satisfy the requirements? > >> > >> > What needs to be done to safely do that in kernel? > >> > >> > > >> > >> > The other approach I've considered is doing a full open()/setns() in > >> > >> > kernel (since the caller already knows its pid) but it sounds like > >> > >> > th
Re: [RFC PATCH 3/4] kmod - add call_usermodehelper_ns() helper
On Wed, 2014-12-03 at 10:49 -0600, Eric W. Biederman wrote: > > >> > Those are the general parameters. > >> > >> It does seem very expensive to keep a thread around for every mount; I'm > >> still trying to find a way around it.. > > > > Yeah, that's not such a good idea. > > > > Several hundred mounts is going to create a big mess for system > > administrators, not to mention the overhead. It's right up there with > > symlinking /etc/mtab to /proc/self/mounts at sites with large direct > > mount maps. > > A thread will cost you maybe 40k. In the grand scheme of things that > really isn't a lot. I agree that it would be nice to do better than > one thread per mount. But I start with a thread as a reference point > as that is the trivial way to get things correct. Sure it has merit because it's straight forward but my criticism isn't about overhead. It's about system administrators annoyance and frustration level when they're trying get things done. For example, with the symlinking of the mount table and even just a few hundred direct automounts, listing the mount table fills the screen with tombs of stuff that really should be hidden (and only listed if requested). That just gets in the way when you're trying desperately to resolve some urgent problem. There is a resource issue as well since so many site administration applications will read the table and, as the number of entries gets larger, so does the time these things take to run. Not having to deal with this on a day to day basis tends to make us forget about these little side issues but I think they are important. Point is, for process listings the issue is almost exactly the same. Ian -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC PATCH 3/4] kmod - add call_usermodehelper_ns() helper
On Mon, 2014-12-01 at 16:56 -0500, Benjamin Coddington wrote: > n Tue, 25 Nov 2014, Eric W. Biederman wrote: > Hi, > > > Ian Kent writes: > > > > > On Tue, 2014-11-25 at 17:19 -0600, Eric W. Biederman wrote: > > >> Ian Kent writes: > > >> > > >> > On Tue, 2014-11-25 at 16:23 -0600, Eric W. Biederman wrote: > > >> >> Oleg Nesterov writes: > > >> >> > > >> >> > On 11/25, Oleg Nesterov wrote: > > >> >> >> > > >> >> >> Let me first apologize, I didn't actually read this series yet. > > >> >> >> > > >> >> >> But I have to admit that so far I do not like this approach... > > >> >> >> probably I am biased. > > >> >> > > > >> >> > Yes. > > >> >> > > > >> >> > And I have another concern... this is mostly a feeling, I can be > > >> >> > easily wrong but: > > >> >> > > > >> >> >> On 11/25, Ian Kent wrote: > > >> >> >> > > > >> >> >> > +static int umh_set_ns(struct subprocess_info *info, struct cred > > >> >> >> > *new) > > >> >> >> > +{ > > >> >> >> > +struct nsproxy *ns = info->data; > > >> >> >> > + > > >> >> >> > +mntns_setfs(ns->mnt_ns); > > >> >> >> > > >> >> >> Firstly, it is not clear to me if we should use the caller's > > >> >> >> ->mnt_ns. > > >> >> >> Let me remind about the coredump. The dumping task can cloned with > > >> >> >> CLONE_NEWNS or it cam do unshare(NEWNS)... but OK, I do not > > >> >> >> understand > > >> >> >> this enough. > > >> >> > > > >> >> > And otoh. If we actually want to use the caller's mnt_ns/namespaces > > >> >> > we > > >> >> > could simply fork/reparent a child which will do execve ? > > >> >> > > >> >> That would certainly be a better approach, and roughly equivalent to > > >> >> what exists here. That would even ensure we remain in the proper > > >> >> cgroups, and lsm context. > > >> >> > > >> >> The practical problem with the approach presented here is that I can > > >> >> hijack any user mode helper I wish, and make it run in any executable > > >> >> I > > >> >> wish as the global root user. > > >> >> > > >> >> Ian if we were to merge this I believe you would win the award for > > >> >> easiest path to a root shell. > > >> > > > >> > LOL, OK, so there's a problem with this. > > >> > > > >> > But, how should a user mode helper execute within a namespace (or more > > >> > specifically within a container)? > > >> > > > >> > Suppose a user mode helper program scans through the pid list and > > >> > somehow picks the correct process pid and then does an > > >> > open()/setns()/execve(). > > >> > > > >> > Does that then satisfy the requirements? > > >> > What needs to be done to safely do that in kernel? > > >> > > > >> > The other approach I've considered is doing a full open()/setns() in > > >> > kernel (since the caller already knows its pid) but it sounds like > > >> > that's not right either. > > >> > > >> The approach we agreed upon with the core dump helper was to provide > > >> enough information that userspace could figure out what was the > > >> appropriate policy and call nsenter/setns. > > > > > > Your recommending I have a look at the core dump helper, that's fine, > > > I'll do that. > > > > I am just describing it because it came up. Core dumps are a much > > easier case than nfs. > > > > Frankly if we can figure out how to run the user mode helpers from the > > kernel with an appropriate context and not involve userspace I think > > that will be better for everyone, as it involves fewer moving parts at > > the end of the day. > > > > >> The only sane approach I can think of in th
Re: [RFC PATCH 3/4] kmod - add call_usermodehelper_ns() helper
On Tue, 2014-11-25 at 17:27 -0600, Eric W. Biederman wrote: > > > How does one correctly set the namespace in user space since each of > > the /proc//ns/ will use a slightly different > > proc_ns_operations install function? > > > > Are we saying that, for example, if open(/proc//ns/pid)/setns() is > > used then the process must not do path lookups if it expects them to be > > within the namespace and restrict itself to pid related system calls > > only and so on for each of the other namespaces? > > In userspace you can only set the pid namespace for new children. You > can never change your own pid namespace. Because actually changing a > processes pid is too nasty to contemplate, or implement and because in a > login daemon context having your first child be the initial process of > the pid namespace is actually what is desirable. > Maybe using the pid namespace was a bad example but now it seems I don't understand the purpose of /proc//ns/pid with the use of setns() either. I wasn't thinking of changing the process pid here at all, as you say from the kernel POV that must not happen, I was thinking of changing an execed userspace process view of pids. Assuming that is valid within a callers namespace (and I believe that's checked along the way) doesn't using this allow the created userspace process to see pids in the view of the given namespace? Isn't that the intended use of this? Ian -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC PATCH 3/4] kmod - add call_usermodehelper_ns() helper
On Wed, 2014-11-26 at 07:50 +0800, Ian Kent wrote: > > > > If we are going to set this stuff up in the kernel we need a reference > > process that we can create children of because what is possible with > > respect to containers keeps changing, and it is extremely error prone to > > figure out what all othe crazy little bits are, and to update everything > > every time someone tweaks the kernel's capabilities. We have kthreadd > > because it was too error prone to scrub a userspace thread of all of the > > userspace bits and make it the equivalent of what kthreadd is today. > > > > Of course it is also rather nice to have something to hang everything > > else on. > > > > In summary we need a reference struct task that is all setup properly > > so that we can create an appropriate kernel thread. > > I'm having trouble understanding what your getting at here but I'm not > that sharp so bear with me. > > When call_usermodehelper() is called it's called from a process that is > within the context within which the execution is required. Umm .. OK, that's probably not quite right either For nfsd I think it's OK but for nfs clients the context is probably that of the caller Whereas the helper to get a key info maybe does need to be called in the context of the caller . > > So what information do we not have available for setup? > > Are you saying that the problem is that when the user mode helper run > thread is invoked we don't have the information available that was > present when call_usermodehelper() was called and that's where the > challenge lies? > > Ian > -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC PATCH 3/4] kmod - add call_usermodehelper_ns() helper
On Tue, 2014-11-25 at 17:19 -0600, Eric W. Biederman wrote: > Ian Kent writes: > > > On Tue, 2014-11-25 at 16:23 -0600, Eric W. Biederman wrote: > >> Oleg Nesterov writes: > >> > >> > On 11/25, Oleg Nesterov wrote: > >> >> > >> >> Let me first apologize, I didn't actually read this series yet. > >> >> > >> >> But I have to admit that so far I do not like this approach... > >> >> probably I am biased. > >> > > >> > Yes. > >> > > >> > And I have another concern... this is mostly a feeling, I can be > >> > easily wrong but: > >> > > >> >> On 11/25, Ian Kent wrote: > >> >> > > >> >> > +static int umh_set_ns(struct subprocess_info *info, struct cred *new) > >> >> > +{ > >> >> > + struct nsproxy *ns = info->data; > >> >> > + > >> >> > + mntns_setfs(ns->mnt_ns); > >> >> > >> >> Firstly, it is not clear to me if we should use the caller's ->mnt_ns. > >> >> Let me remind about the coredump. The dumping task can cloned with > >> >> CLONE_NEWNS or it cam do unshare(NEWNS)... but OK, I do not understand > >> >> this enough. > >> > > >> > And otoh. If we actually want to use the caller's mnt_ns/namespaces we > >> > could simply fork/reparent a child which will do execve ? > >> > >> That would certainly be a better approach, and roughly equivalent to > >> what exists here. That would even ensure we remain in the proper > >> cgroups, and lsm context. > >> > >> The practical problem with the approach presented here is that I can > >> hijack any user mode helper I wish, and make it run in any executable I > >> wish as the global root user. > >> > >> Ian if we were to merge this I believe you would win the award for > >> easiest path to a root shell. > > > > LOL, OK, so there's a problem with this. > > > > But, how should a user mode helper execute within a namespace (or more > > specifically within a container)? > > > > Suppose a user mode helper program scans through the pid list and > > somehow picks the correct process pid and then does an > > open()/setns()/execve(). > > > > Does that then satisfy the requirements? > > What needs to be done to safely do that in kernel? > > > > The other approach I've considered is doing a full open()/setns() in > > kernel (since the caller already knows its pid) but it sounds like > > that's not right either. > > The approach we agreed upon with the core dump helper was to provide > enough information that userspace could figure out what was the > appropriate policy and call nsenter/setns. Your recommending I have a look at the core dump helper, that's fine, I'll do that. > > The only sane approach I can think of in the context of nfs is to fork > a kernel thread at mount time that has all of the appropriate context > because it was captured from the privileged mounting process, and use > that kernel as the equivalent of kthreadd. > > There may be some intermediate ground where we capture things or we use > the init process of the pid namespace (captured at mount time) as our > template/reference process. > > If we are going to set this stuff up in the kernel we need a reference > process that we can create children of because what is possible with > respect to containers keeps changing, and it is extremely error prone to > figure out what all othe crazy little bits are, and to update everything > every time someone tweaks the kernel's capabilities. We have kthreadd > because it was too error prone to scrub a userspace thread of all of the > userspace bits and make it the equivalent of what kthreadd is today. > > Of course it is also rather nice to have something to hang everything > else on. > > In summary we need a reference struct task that is all setup properly > so that we can create an appropriate kernel thread. I'm having trouble understanding what your getting at here but I'm not that sharp so bear with me. When call_usermodehelper() is called it's called from a process that is within the context within which the execution is required. So what information do we not have available for setup? Are you saying that the problem is that when the user mode helper run thread is invoked we don't have the information available that was present when call_usermodehelper() was called and that's where the challenge lies? Ian -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC PATCH 3/4] kmod - add call_usermodehelper_ns() helper
On Tue, 2014-11-25 at 23:06 +0100, Oleg Nesterov wrote: > On 11/25, Oleg Nesterov wrote: > > > > Let me first apologize, I didn't actually read this series yet. > > > > But I have to admit that so far I do not like this approach... > > probably I am biased. > > Yes. > > And I have another concern... this is mostly a feeling, I can be > easily wrong but: > > > On 11/25, Ian Kent wrote: > > > > > > +static int umh_set_ns(struct subprocess_info *info, struct cred *new) > > > +{ > > > + struct nsproxy *ns = info->data; > > > + > > > + mntns_setfs(ns->mnt_ns); > > > > Firstly, it is not clear to me if we should use the caller's ->mnt_ns. > > Let me remind about the coredump. The dumping task can cloned with > > CLONE_NEWNS or it cam do unshare(NEWNS)... but OK, I do not understand > > this enough. > > And otoh. If we actually want to use the caller's mnt_ns/namespaces we > could simply fork/reparent a child which will do execve ? Are you saying that the user space program should be modified to do this? Ian -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC PATCH 3/4] kmod - add call_usermodehelper_ns() helper
On Tue, 2014-11-25 at 16:23 -0600, Eric W. Biederman wrote: > Oleg Nesterov writes: > > > On 11/25, Oleg Nesterov wrote: > >> > >> Let me first apologize, I didn't actually read this series yet. > >> > >> But I have to admit that so far I do not like this approach... > >> probably I am biased. > > > > Yes. > > > > And I have another concern... this is mostly a feeling, I can be > > easily wrong but: > > > >> On 11/25, Ian Kent wrote: > >> > > >> > +static int umh_set_ns(struct subprocess_info *info, struct cred *new) > >> > +{ > >> > +struct nsproxy *ns = info->data; > >> > + > >> > +mntns_setfs(ns->mnt_ns); > >> > >> Firstly, it is not clear to me if we should use the caller's ->mnt_ns. > >> Let me remind about the coredump. The dumping task can cloned with > >> CLONE_NEWNS or it cam do unshare(NEWNS)... but OK, I do not understand > >> this enough. > > > > And otoh. If we actually want to use the caller's mnt_ns/namespaces we > > could simply fork/reparent a child which will do execve ? > > That would certainly be a better approach, and roughly equivalent to > what exists here. That would even ensure we remain in the proper > cgroups, and lsm context. > > The practical problem with the approach presented here is that I can > hijack any user mode helper I wish, and make it run in any executable I > wish as the global root user. > > Ian if we were to merge this I believe you would win the award for > easiest path to a root shell. LOL, OK, so there's a problem with this. But, how should a user mode helper execute within a namespace (or more specifically within a container)? Suppose a user mode helper program scans through the pid list and somehow picks the correct process pid and then does an open()/setns()/execve(). Does that then satisfy the requirements? What needs to be done to safely do that in kernel? The other approach I've considered is doing a full open()/setns() in kernel (since the caller already knows its pid) but it sounds like that's not right either. Ian -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC PATCH 3/4] kmod - add call_usermodehelper_ns() helper
On Tue, 2014-11-25 at 22:52 +0100, Oleg Nesterov wrote: > Let me first apologize, I didn't actually read this series yet. > > But I have to admit that so far I do not like this approach... > probably I am biased. Oleg, thanks for your comments. > > On 11/25, Ian Kent wrote: > > > > The call_usermodehelper() function executes all binaries in the > > global "init" root context. This doesn't allow a binary to be run > > within the callers namespace (aka. a container). > > Please see below. > > > Both containerized NFS client and NFS server need the ability to > > execute a binary within their container. To do this create a new > > nsproxy within the callers' context so it can be used for setup > > prior to calling do_execve() from the user mode helper thread > > runner. > > and probably we also need this for coredump helpers, we want them > to be per-namespace. To save me some time could you point me to some of the related code please. I don't normally play in that area. > > > +static int umh_set_ns(struct subprocess_info *info, struct cred *new) > > +{ > > + struct nsproxy *ns = info->data; > > + > > + mntns_setfs(ns->mnt_ns); > > Firstly, it is not clear to me if we should use the caller's ->mnt_ns. > Let me remind about the coredump. The dumping task can cloned with > CLONE_NEWNS or it cam do unshare(NEWNS)... but OK, I do not understand > this enough. > > > > + switch_task_namespaces(current, ns); > > This doesn't look sane because this won't switch task_active_pid_ns(). I wondered about that too but I didn't design the open()/setns() interface and TBH I've been wondering how he hell it is supposed to work because of exactly this. The statement amounts to saying that the fd = open(/proc//ns/mnt); setns(fd); won't set the namespace properly but the documentation I've seen so far (there's probably more that I need to see, I'll look further) implies this is sufficient. How does one correctly set the namespace in user space since each of the /proc//ns/ will use a slightly different proc_ns_operations install function? Are we saying that, for example, if open(/proc//ns/pid)/setns() is used then the process must not do path lookups if it expects them to be within the namespace and restrict itself to pid related system calls only and so on for each of the other namespaces? Or is it assumed that userspace will do open(/proc//ns/)/setns()/close() every time it makes systems calls that rely on a specific type of namespace? > > And this reminds me another discussion, please look at > http://marc.info/?l=linux-kernel&m=138479570926192 > > Once again, this is just an idea to provoke more discussion. I am starting > to think that perhaps we need pid_ns->umh_helper (init by default). And > PR_SET_NS_UMH_HELPER. Yeah, I'll need to digest that for a while. Ian -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[RFC PATCH 2/4] nsproxy - make create_new_namespaces() non-static
create_new_namespaces() will be needed by call_usermodehelper_ns() and call_usermodehelper_keys() for namespace restricted execution. Signed-off-by: Ian Kent Signed-off-by: Benjamin Coddington Cc: Al Viro Cc: J. Bruce Fields Cc: David Howells Cc: Trond Myklebust Cc: Stanislav Kinsbursky Cc: Oleg Nesterov Cc: Eric W. Biederman --- include/linux/nsproxy.h |3 +++ kernel/nsproxy.c|2 +- 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/include/linux/nsproxy.h b/include/linux/nsproxy.h index 35fa08f..dfe7dda 100644 --- a/include/linux/nsproxy.h +++ b/include/linux/nsproxy.h @@ -62,6 +62,9 @@ extern struct nsproxy init_nsproxy; * */ +struct nsproxy *create_new_namespaces(unsigned long flags, + struct task_struct *tsk, struct user_namespace *user_ns, + struct fs_struct *new_fs); int copy_namespaces(unsigned long flags, struct task_struct *tsk); void exit_task_namespaces(struct task_struct *tsk); void switch_task_namespaces(struct task_struct *tsk, struct nsproxy *new); diff --git a/kernel/nsproxy.c b/kernel/nsproxy.c index ef42d0a..dcfbb13 100644 --- a/kernel/nsproxy.c +++ b/kernel/nsproxy.c @@ -56,7 +56,7 @@ static inline struct nsproxy *create_nsproxy(void) * Return the newly created nsproxy. Do not attach this to the task, * leave it to the caller to do proper locking and attach it to task. */ -static struct nsproxy *create_new_namespaces(unsigned long flags, +struct nsproxy *create_new_namespaces(unsigned long flags, struct task_struct *tsk, struct user_namespace *user_ns, struct fs_struct *new_fs) { -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[RFC PATCH 4/4] KEYS: exec request-key within the requesting task's namespace
From: Benjamin Coddington Copy the current task's namespaces into the request-key userspace helper to restrict contained processes from executing key instantiation processes outside their containers. Signed-off-by: Benjamin Coddington Signed-off-by: Ian Kent Cc: Al Viro Cc: J. Bruce Fields Cc: David Howells Cc: Trond Myklebust Cc: Stanislav Kinsbursky Cc: Oleg Nesterov Cc: Eric W. Biederman --- security/keys/request_key.c | 45 +-- 1 file changed, 39 insertions(+), 6 deletions(-) diff --git a/security/keys/request_key.c b/security/keys/request_key.c index bb4337c..b03feec 100644 --- a/security/keys/request_key.c +++ b/security/keys/request_key.c @@ -17,6 +17,8 @@ #include #include #include +#include +#include #include "internal.h" #define key_negative_timeout 60 /* default timeout on a negative key's existence */ @@ -46,6 +48,11 @@ void complete_request_key(struct key_construction *cons, int error) } EXPORT_SYMBOL(complete_request_key); +struct request_key_info { + struct key *keyring; + struct nsproxy *nsproxy; +}; + /* * Initialise a usermode helper that is going to have a specific session * keyring. @@ -55,9 +62,14 @@ EXPORT_SYMBOL(complete_request_key); */ static int umh_keys_init(struct subprocess_info *info, struct cred *cred) { - struct key *keyring = info->data; + struct request_key_info *rki = info->data; + struct nsproxy *ns = rki->nsproxy; - return install_session_keyring_to_cred(cred, keyring); + if (ns) { + mntns_setfs(ns->mnt_ns); + switch_task_namespaces(current, ns); + } + return install_session_keyring_to_cred(cred, rki->keyring); } /* @@ -65,8 +77,9 @@ static int umh_keys_init(struct subprocess_info *info, struct cred *cred) */ static void umh_keys_cleanup(struct subprocess_info *info) { - struct key *keyring = info->data; - key_put(keyring); + struct request_key_info *rki = info->data; + key_put(rki->keyring); + kfree(rki); } /* @@ -76,12 +89,32 @@ static int call_usermodehelper_keys(char *path, char **argv, char **envp, struct key *session_keyring, int wait) { struct subprocess_info *info; + struct request_key_info *rki; + struct nsproxy *ns; + + ns = umh_open_ns(); + if (ns && IS_ERR(ns)) + return PTR_ERR(ns); + + rki = kmalloc(sizeof(*rki), GFP_KERNEL); + if (!rki) { + if (ns) + free_nsproxy(ns); + return -ENOMEM; + } + + rki->keyring = session_keyring; + rki->nsproxy = ns; info = call_usermodehelper_setup(path, argv, envp, GFP_KERNEL, umh_keys_init, umh_keys_cleanup, - session_keyring); - if (!info) + rki); + if (!info) { + if (ns) + free_nsproxy(ns); + kfree(rki); return -ENOMEM; + } key_get(session_keyring); return call_usermodehelper_exec(info, wait); -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[RFC PATCH 3/4] kmod - add call_usermodehelper_ns() helper
The call_usermodehelper() function executes all binaries in the global "init" root context. This doesn't allow a binary to be run within the callers namespace (aka. a container). So create a new function call_usermodehelper_ns() to do this. Both containerized NFS client and NFS server need the ability to execute a binary within their container. To do this create a new nsproxy within the callers' context so it can be used for setup prior to calling do_execve() from the user mode helper thread runner. Signed-off-by: Ian Kent Signed-off-by: Benjamin Coddington Cc: Al Viro Cc: J. Bruce Fields Cc: David Howells Cc: Trond Myklebust Cc: Stanislav Kinsbursky Cc: Oleg Nesterov Cc: Eric W. Biederman --- include/linux/kmod.h | 17 + kernel/kmod.c| 39 +++ 2 files changed, 56 insertions(+) diff --git a/include/linux/kmod.h b/include/linux/kmod.h index 0555cc6..fd5509a 100644 --- a/include/linux/kmod.h +++ b/include/linux/kmod.h @@ -69,6 +69,23 @@ struct subprocess_info { extern int call_usermodehelper(char *path, char **argv, char **envp, int wait); +#if !defined(CONFIG_PROC_FS) || !defined(CONFIG_NAMESPACES) +inline struct nsproxy *umh_open_ns(void) +{ + return NULL; +} + +inline int +call_usermodehelper_ns(char *path, char **argv, char **envp, int wait) +{ + return -ENOTSUP; +} +#else +extern struct nsproxy *umh_open_ns(void); +extern int +call_usermodehelper_ns(char *path, char **argv, char **envp, int wait); +#endif + extern struct subprocess_info * call_usermodehelper_setup(char *path, char **argv, char **envp, gfp_t gfp_mask, int (*init)(struct subprocess_info *info, struct cred *new), diff --git a/kernel/kmod.c b/kernel/kmod.c index 80f7a6d..0ddcfbb 100644 --- a/kernel/kmod.c +++ b/kernel/kmod.c @@ -39,6 +39,7 @@ #include #include #include +#include #include #include @@ -642,6 +643,44 @@ int call_usermodehelper(char *path, char **argv, char **envp, int wait) } EXPORT_SYMBOL(call_usermodehelper); +#if defined(CONFIG_PROC_FS) && defined(CONFIG_NAMESPACES) +static int umh_set_ns(struct subprocess_info *info, struct cred *new) +{ + struct nsproxy *ns = info->data; + + mntns_setfs(ns->mnt_ns); + switch_task_namespaces(current, ns); + return 0; +} + +struct nsproxy *umh_open_ns(void) +{ + return create_new_namespaces(0, current, current_user_ns(), current->fs); +} + +/* Call a usermode helper to execute within current namespace. */ +int call_usermodehelper_ns(char *path, char **argv, char **envp, int wait) +{ + struct subprocess_info *info; + struct nsproxy *ns; + gfp_t gfp_mask = (wait == UMH_NO_WAIT) ? GFP_ATOMIC : GFP_KERNEL; + + ns = umh_open_ns(); + if (IS_ERR(ns)) + return PTR_ERR(ns); + + info = call_usermodehelper_setup(path, argv, envp, +gfp_mask, umh_set_ns, NULL, ns); + if (!info) { + free_nsproxy(ns); + return -ENOMEM; + } + + return call_usermodehelper_exec(info, wait); +} +EXPORT_SYMBOL(call_usermodehelper_ns); +#endif + static int proc_cap_handler(struct ctl_table *table, int write, void __user *buffer, size_t *lenp, loff_t *ppos) { -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[RFC PATCH 1/4] vfs - fs/namespaces.c: break out mntns_setfs() from mntns_install()
Some users of kmod.c's usermodehelper need to setup their fs_struct root and pwd based on the callers namespaces after the kernel thread runner has created a new process but before do_execve() is called. Break out the fs_struct portion of mntns_install so it can be used for this purpose. Signed-off-by: Benjamin Coddington Signed-off-by: Ian Kent Cc: Al Viro Cc: J. Bruce Fields Cc: David Howells Cc: Trond Myklebust Cc: Stanislav Kinsbursky Cc: Oleg Nesterov Cc: Eric W. Biederman --- fs/namespace.c| 41 + include/linux/mount.h |1 + 2 files changed, 30 insertions(+), 12 deletions(-) diff --git a/fs/namespace.c b/fs/namespace.c index 5b66b2b..3cdbb9e 100644 --- a/fs/namespace.c +++ b/fs/namespace.c @@ -3165,11 +3165,38 @@ static void mntns_put(void *ns) put_mnt_ns(ns); } +/* + * Set fs root and pwd for a subsequent call to do_execve(). + * + * This assumes that an nsproxy has been created within the + * namespace context that is the target for the exec so that + * mnt_ns is already set. Additionally, since the nsproxy is + * created within the requesting namespace context the security + * checks of mntns_install() aren't required. + */ +void mntns_setfs(struct mnt_namespace *mnt_ns) +{ + struct fs_struct *fs = current->fs; + struct path root; + + /* Find the root */ + root.mnt= &mnt_ns->root->mnt; + root.dentry = mnt_ns->root->mnt.mnt_root; + path_get(&root); + while(d_mountpoint(root.dentry) && follow_down_one(&root)) + ; + + /* Update the pwd and root */ + set_fs_pwd(fs, &root); + set_fs_root(fs, &root); + + path_put(&root); +} + static int mntns_install(struct nsproxy *nsproxy, void *ns) { struct fs_struct *fs = current->fs; struct mnt_namespace *mnt_ns = ns; - struct path root; if (!ns_capable(mnt_ns->user_ns, CAP_SYS_ADMIN) || !ns_capable(current_user_ns(), CAP_SYS_CHROOT) || @@ -3183,18 +3210,8 @@ static int mntns_install(struct nsproxy *nsproxy, void *ns) put_mnt_ns(nsproxy->mnt_ns); nsproxy->mnt_ns = mnt_ns; - /* Find the root */ - root.mnt= &mnt_ns->root->mnt; - root.dentry = mnt_ns->root->mnt.mnt_root; - path_get(&root); - while(d_mountpoint(root.dentry) && follow_down_one(&root)) - ; - - /* Update the pwd and root */ - set_fs_pwd(fs, &root); - set_fs_root(fs, &root); + mntns_setfs(mnt_ns); - path_put(&root); return 0; } diff --git a/include/linux/mount.h b/include/linux/mount.h index c2c561d..a9f6548 100644 --- a/include/linux/mount.h +++ b/include/linux/mount.h @@ -80,6 +80,7 @@ extern void mntput(struct vfsmount *mnt); extern struct vfsmount *mntget(struct vfsmount *mnt); extern struct vfsmount *mnt_clone_internal(struct path *path); extern int __mnt_is_readonly(struct vfsmount *mnt); +void mntns_setfs(struct mnt_namespace *mnt_ns); struct path; extern struct vfsmount *clone_private_mount(struct path *path); -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[RFC PATCH 0/4] Namespace contrained helper execution
Hi all, Some time ago an attempt was made to update call_usermodehelper() to execute within it's namespace. Comments at the time were basically that the approach didn't go nearly far enough to constrain the process. This series attempts to remedy that by taking care to create an appropriate namespace environment then switch to it and setup fs_struct for path walking prior to the user mode helper thread runner calling do_execve(). Please review and comment on the patch series. Ian --- Benjamin Coddington (1): KEYS: exec request-key within the requesting task's namespace Ian Kent (3): vfs - fs/namespaces.c: break out mntns_setfs() from mntns_install() nsproxy - make create_new_namespaces() non-static kmod - add call_usermodehelper_ns() helper fs/namespace.c | 41 --- include/linux/kmod.h| 17 include/linux/mount.h |1 + include/linux/nsproxy.h |3 +++ kernel/kmod.c | 39 + kernel/nsproxy.c|2 +- security/keys/request_key.c | 45 +-- 7 files changed, 129 insertions(+), 19 deletions(-) -- Signature -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/3] autofs - fix log print messages
On Thu, 2014-11-13 at 08:10 +0800, Ian Kent wrote: > > > > The tree which these patches were based on seem to have more than a > > plain rename. For example I get > > Right, looks like something has gone missing along the way. Ahh .. I get it now. > The tree the patches are against is just a local clone of the Linus tree > with the patches I'd sent previously applied. > > Folding these recent patches into the original series and sending one > series only that applies on the current Linus tree should sort out the > problem. > > > > > --- fs/autofs4/autofs_i.h > > +++ fs/autofs4/autofs_i.h Looks like the patches I sent on 3rd Dec, which is what Joe commented on, were missed. It's probably better for me to start over once its decided how I'll be sending the patches. Ian -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/3] autofs - fix log print messages
On Wed, 2014-11-12 at 15:07 -0800, Andrew Morton wrote: > On Tue, 11 Nov 2014 14:01:52 +0800 Ian Kent wrote: > > > I guess I could make a kernel.org tree but, apart from this recent > > rename autofs4 -> autofs, the number of autofs changes hasn't warranted > > maintaining a separate tree for quite a while. > > > > It looks like the place holder under /pub/scm/linux/kernel/git/raven is > > no longer present so I expect I'd need to request that be setup. > > > > That's why I just send my patches to Andrew to include them in -next. > > > > Andrew, thoughts? > > Various options. > > a) redo these patches against mainline and I merge them into >3.19-rc1. Shortly after that you send me the big rename as plain >old patches or, preferably, you send Linus a git pull request. For >-rc2. Given that something has gone missing along the way I'll fold these latest patches into my original rename (and cleanup) series and resend or whatever I end up doing. > > b) You run a git tree for a while. So that would mean doing what Joe recommended and asking Stephen Rothwell to accept pull requests for my tree, and then send the pull request to Linus after some reasonable soak time, correct? > > The tree which these patches were based on seem to have more than a > plain rename. For example I get Right, looks like something has gone missing along the way. The tree the patches are against is just a local clone of the Linus tree with the patches I'd sent previously applied. Folding these recent patches into the original series and sending one series only that applies on the current Linus tree should sort out the problem. > > --- fs/autofs4/autofs_i.h > +++ fs/autofs4/autofs_i.h > @@ -37,15 +37,15 @@ > /* #define DEBUG */ > > #define DPRINTK(fmt, ...)\ > - pr_debug("pid %d: %s: " fmt "\n", \ > + pr_debug(KBUILD_MODNAME ":pid:%d:%s: " fmt "\n",\ > current->pid, __func__, ##__VA_ARGS__) > > #define AUTOFS_WARN(fmt, ...)\ > - pr_warn("pid %d: %s: " fmt "\n",\ > + pr_warn(KBUILD_MODNAME ":pid:%d:%s: " fmt "\n", \ > current->pid, __func__, ##__VA_ARGS__) > > #define AUTOFS_ERROR(fmt, ...) \ > - pr_err("pid %d: %s: " fmt "\n", \ > + pr_err(KBUILD_MODNAME ":pid:%d:%s: " fmt "\n", \ > current->pid, __func__, ##__VA_ARGS__) > > /* > > > But I'm seeing > > #define DPRINTK(fmt, ...) \ > pr_debug("pid %d: %s: " fmt "\n", \ > current->pid, __func__, ##__VA_ARGS__) > > #define AUTOFS_WARN(fmt, ...) \ > printk(KERN_WARNING "pid %d: %s: " fmt "\n",\ > current->pid, __func__, ##__VA_ARGS__) > > #define AUTOFS_ERROR(fmt, ...)\ > printk(KERN_ERR "pid %d: %s: " fmt "\n",\ > current->pid, __func__, ##__VA_ARGS__) > > /* Unified info structure. This is pointed to by both the dentry and > > ie: the comment layout was changed. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/3] autofs - fix log print messages
On Mon, 2014-11-10 at 21:49 -0800, Joe Perches wrote: > On Tue, 2014-11-11 at 13:37 +0800, Ian Kent wrote: > > On Mon, 2014-11-10 at 19:25 -0800, Joe Perches wrote: > > > On Tue, 2014-11-11 at 09:29 +0800, Ian Kent wrote: > > > > Fix some inconsistencies in log print output and ensure that > > > > the module name is included in all log prints. > > > > > > > > Signed-off-by: Ian Kent > > > > Cc: Joe Perches > > > > --- > > > > fs/autofs/autofs_i.h |6 +++--- > > > > fs/autofs/dev-ioctl.c |2 +- > > > > fs/autofs/inode.c | 10 +- > > > > fs/autofs/waitq.c |4 ++-- > > > > 4 files changed, 11 insertions(+), 11 deletions(-) > > > > > > What tree are these patches made against? > > > > They live in an StGit patch queue against the current Linus tree. > > They depend on the prior series to rename autofs4 to autofs. > > Hey Ian. > > Maybe you could add the git tree location to > the MAINTAINERS entry: > > MAINTAINERS-KERNEL AUTOMOUNTER v4 (AUTOFS4) > MAINTAINERS-M: Ian Kent > MAINTAINERS-L: aut...@vger.kernel.org > MAINTAINERS-S: Maintained > MAINTAINERS:F: fs/autofs4/ ... and I forgot about updating that fs/autofs4/ in MAINTAINERS too, ha, that'll need another patch. Ian -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/3] autofs - fix log print messages
On Mon, 2014-11-10 at 21:49 -0800, Joe Perches wrote: > On Tue, 2014-11-11 at 13:37 +0800, Ian Kent wrote: > > On Mon, 2014-11-10 at 19:25 -0800, Joe Perches wrote: > > > On Tue, 2014-11-11 at 09:29 +0800, Ian Kent wrote: > > > > Fix some inconsistencies in log print output and ensure that > > > > the module name is included in all log prints. > > > > > > > > Signed-off-by: Ian Kent > > > > Cc: Joe Perches > > > > --- > > > > fs/autofs/autofs_i.h |6 +++--- > > > > fs/autofs/dev-ioctl.c |2 +- > > > > fs/autofs/inode.c | 10 +- > > > > fs/autofs/waitq.c |4 ++-- > > > > 4 files changed, 11 insertions(+), 11 deletions(-) > > > > > > What tree are these patches made against? > > > > They live in an StGit patch queue against the current Linus tree. > > They depend on the prior series to rename autofs4 to autofs. > > Hey Ian. > > Maybe you could add the git tree location to > the MAINTAINERS entry: > > MAINTAINERS-KERNEL AUTOMOUNTER v4 (AUTOFS4) > MAINTAINERS-M: Ian Kent > MAINTAINERS-L: aut...@vger.kernel.org > MAINTAINERS-S: Maintained > MAINTAINERS:F: fs/autofs4/ > > T:git git://somewhere_or_other... > > and maybe you also could get Stephen Rothwell > to add that tree to -next Oh, you meant what public tree. I guess I could make a kernel.org tree but, apart from this recent rename autofs4 -> autofs, the number of autofs changes hasn't warranted maintaining a separate tree for quite a while. It looks like the place holder under /pub/scm/linux/kernel/git/raven is no longer present so I expect I'd need to request that be setup. That's why I just send my patches to Andrew to include them in -next. Andrew, thoughts? Ian -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/3] autofs - fix log print messages
On Mon, 2014-11-10 at 19:25 -0800, Joe Perches wrote: > On Tue, 2014-11-11 at 09:29 +0800, Ian Kent wrote: > > Fix some inconsistencies in log print output and ensure that > > the module name is included in all log prints. > > > > Signed-off-by: Ian Kent > > Cc: Joe Perches > > --- > > fs/autofs/autofs_i.h |6 +++--- > > fs/autofs/dev-ioctl.c |2 +- > > fs/autofs/inode.c | 10 +- > > fs/autofs/waitq.c |4 ++-- > > 4 files changed, 11 insertions(+), 11 deletions(-) > > What tree are these patches made against? They live in an StGit patch queue against the current Linus tree. They depend on the prior series to rename autofs4 to autofs. Ian -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH 2/3] autofs - change log prints to not insert newline
Common kernel coding practice is to include the newline of log prints within the log text rather than hidden away in a macro. To avoid introducing inconsistencies as changes are made change the log macros to not include the newline. Signed-off-by: Ian Kent Cc: Joe Perches --- fs/autofs/autofs_i.h |6 +++--- fs/autofs/dev-ioctl.c | 14 +++--- fs/autofs/expire.c| 24 fs/autofs/inode.c | 16 fs/autofs/root.c | 26 +- fs/autofs/waitq.c | 10 +- 6 files changed, 48 insertions(+), 48 deletions(-) diff --git a/fs/autofs/autofs_i.h b/fs/autofs/autofs_i.h index fd93fdc..84d5060 100644 --- a/fs/autofs/autofs_i.h +++ b/fs/autofs/autofs_i.h @@ -37,15 +37,15 @@ /* #define DEBUG */ #define DPRINTK(fmt, ...) \ - pr_debug(KBUILD_MODNAME ":pid:%d:%s: " fmt "\n",\ + pr_debug(KBUILD_MODNAME ":pid:%d:%s: " fmt, \ current->pid, __func__, ##__VA_ARGS__) #define AUTOFS_WARN(fmt, ...) \ - pr_warn(KBUILD_MODNAME ":pid:%d:%s: " fmt "\n", \ + pr_warn(KBUILD_MODNAME ":pid:%d:%s: " fmt, \ current->pid, __func__, ##__VA_ARGS__) #define AUTOFS_ERROR(fmt, ...) \ - pr_err(KBUILD_MODNAME ":pid:%d:%s: " fmt "\n", \ + pr_err(KBUILD_MODNAME ":pid:%d:%s: " fmt, \ current->pid, __func__, ##__VA_ARGS__) /* diff --git a/fs/autofs/dev-ioctl.c b/fs/autofs/dev-ioctl.c index eb16fe4..15ce91b 100644 --- a/fs/autofs/dev-ioctl.c +++ b/fs/autofs/dev-ioctl.c @@ -75,7 +75,7 @@ static int check_dev_ioctl_version(int cmd, struct autofs_dev_ioctl *param) if ((AUTOFS_DEV_IOCTL_VERSION_MAJOR != param->ver_major) || (AUTOFS_DEV_IOCTL_VERSION_MINOR < param->ver_minor)) { AUTOFS_WARN("ioctl control interface version mismatch: " -"kernel(%u.%u), user(%u.%u), cmd(%d)", +"kernel(%u.%u), user(%u.%u), cmd(%d)\n", AUTOFS_DEV_IOCTL_VERSION_MAJOR, AUTOFS_DEV_IOCTL_VERSION_MINOR, param->ver_major, param->ver_minor, cmd); @@ -126,7 +126,7 @@ static int validate_dev_ioctl(int cmd, struct autofs_dev_ioctl *param) err = check_dev_ioctl_version(cmd, param); if (err) { AUTOFS_WARN("invalid device control module version " -"supplied for cmd(0x%08x)", cmd); +"supplied for cmd(0x%08x)\n", cmd); goto out; } @@ -134,14 +134,14 @@ static int validate_dev_ioctl(int cmd, struct autofs_dev_ioctl *param) err = invalid_str(param->path, param->size - sizeof(*param)); if (err) { AUTOFS_WARN( - "path string terminator missing for cmd(0x%08x)", + "path string terminator missing for cmd(0x%08x)\n", cmd); goto out; } err = check_name(param->path); if (err) { - AUTOFS_WARN("invalid path supplied for cmd(0x%08x)", + AUTOFS_WARN("invalid path supplied for cmd(0x%08x)\n", cmd); goto out; } @@ -369,7 +369,7 @@ static int autofs_dev_ioctl_setpipefd(struct file *fp, new_pid = get_task_pid(current, PIDTYPE_PGID); if (ns_of_pid(new_pid) != ns_of_pid(sbi->oz_pgrp)) { - AUTOFS_WARN("not allowed to change PID namespace"); + AUTOFS_WARN("not allowed to change PID namespace\n"); err = -EINVAL; goto out; } @@ -657,7 +657,7 @@ static int _autofs_dev_ioctl(unsigned int command, fn = lookup_dev_ioctl(cmd); if (!fn) { - AUTOFS_WARN("unknown command 0x%08x", command); + AUTOFS_WARN("unknown command 0x%08x\n", command); return -ENOTTY; } @@ -750,7 +750,7 @@ int __init autofs_dev_ioctl_init(void) r = misc_register(&_autofs_dev_ioctl_misc); if (r) { - AUTOFS_ERROR("misc_register failed for control device"); + AUTOFS_ERROR("misc_register failed for control device\n"); return r; } diff --git a/fs/autofs/expire.c b/fs/autofs/expire.c index c3090b4..99f51cf 100644 --- a/fs/autofs/expire.c +++ b/fs/autofs/expire.c @@ -38,7 +38,7 @@ static int autofs_mount_busy(struct vfsmount *mnt, struct dentry
[PATCH 3/3] autofs - use pr_xxx() calls directly for logging
From: Ian Kent --- fs/autofs/autofs_i.h | 15 --- fs/autofs/dev-ioctl.c | 26 +- fs/autofs/expire.c| 42 +- fs/autofs/inode.c | 17 - fs/autofs/root.c | 44 ++-- fs/autofs/waitq.c | 24 6 files changed, 80 insertions(+), 88 deletions(-) diff --git a/fs/autofs/autofs_i.h b/fs/autofs/autofs_i.h index 84d5060..1015999 100644 --- a/fs/autofs/autofs_i.h +++ b/fs/autofs/autofs_i.h @@ -36,17 +36,10 @@ /* #define DEBUG */ -#define DPRINTK(fmt, ...) \ - pr_debug(KBUILD_MODNAME ":pid:%d:%s: " fmt, \ - current->pid, __func__, ##__VA_ARGS__) - -#define AUTOFS_WARN(fmt, ...) \ - pr_warn(KBUILD_MODNAME ":pid:%d:%s: " fmt, \ - current->pid, __func__, ##__VA_ARGS__) - -#define AUTOFS_ERROR(fmt, ...) \ - pr_err(KBUILD_MODNAME ":pid:%d:%s: " fmt, \ - current->pid, __func__, ##__VA_ARGS__) +#ifdef pr_fmt +#undef pr_fmt +#endif +#define pr_fmt(fmt) KBUILD_MODNAME ":pid:%d:%s: " fmt, current->pid, __func__ /* * Unified info structure. This is pointed to by both the dentry and diff --git a/fs/autofs/dev-ioctl.c b/fs/autofs/dev-ioctl.c index 15ce91b..db04512 100644 --- a/fs/autofs/dev-ioctl.c +++ b/fs/autofs/dev-ioctl.c @@ -74,11 +74,11 @@ static int check_dev_ioctl_version(int cmd, struct autofs_dev_ioctl *param) if ((AUTOFS_DEV_IOCTL_VERSION_MAJOR != param->ver_major) || (AUTOFS_DEV_IOCTL_VERSION_MINOR < param->ver_minor)) { - AUTOFS_WARN("ioctl control interface version mismatch: " -"kernel(%u.%u), user(%u.%u), cmd(%d)\n", -AUTOFS_DEV_IOCTL_VERSION_MAJOR, -AUTOFS_DEV_IOCTL_VERSION_MINOR, -param->ver_major, param->ver_minor, cmd); + pr_warn("ioctl control interface version mismatch: " + "kernel(%u.%u), user(%u.%u), cmd(%d)\n", + AUTOFS_DEV_IOCTL_VERSION_MAJOR, + AUTOFS_DEV_IOCTL_VERSION_MINOR, + param->ver_major, param->ver_minor, cmd); err = -EINVAL; } @@ -125,15 +125,15 @@ static int validate_dev_ioctl(int cmd, struct autofs_dev_ioctl *param) err = check_dev_ioctl_version(cmd, param); if (err) { - AUTOFS_WARN("invalid device control module version " -"supplied for cmd(0x%08x)\n", cmd); + pr_warn("invalid device control module version " + "supplied for cmd(0x%08x)\n", cmd); goto out; } if (param->size > sizeof(*param)) { err = invalid_str(param->path, param->size - sizeof(*param)); if (err) { - AUTOFS_WARN( + pr_warn( "path string terminator missing for cmd(0x%08x)\n", cmd); goto out; @@ -141,8 +141,8 @@ static int validate_dev_ioctl(int cmd, struct autofs_dev_ioctl *param) err = check_name(param->path); if (err) { - AUTOFS_WARN("invalid path supplied for cmd(0x%08x)\n", - cmd); + pr_warn("invalid path supplied for cmd(0x%08x)\n", + cmd); goto out; } } @@ -369,7 +369,7 @@ static int autofs_dev_ioctl_setpipefd(struct file *fp, new_pid = get_task_pid(current, PIDTYPE_PGID); if (ns_of_pid(new_pid) != ns_of_pid(sbi->oz_pgrp)) { - AUTOFS_WARN("not allowed to change PID namespace\n"); + pr_warn("not allowed to change PID namespace\n"); err = -EINVAL; goto out; } @@ -657,7 +657,7 @@ static int _autofs_dev_ioctl(unsigned int command, fn = lookup_dev_ioctl(cmd); if (!fn) { - AUTOFS_WARN("unknown command 0x%08x\n", command); + pr_warn("unknown command 0x%08x\n", command); return -ENOTTY; } @@ -750,7 +750,7 @@ int __init autofs_dev_ioctl_init(void) r = misc_register(&_autofs_dev_ioctl_misc); if (r) { - AUTOFS_ERROR("misc_register failed for control device\n"); + pr_err("misc_register failed for control device\n"); return r; } diff --git a/fs/a
[PATCH 1/3] autofs - fix log print messages
Fix some inconsistencies in log print output and ensure that the module name is included in all log prints. Signed-off-by: Ian Kent Cc: Joe Perches --- fs/autofs/autofs_i.h |6 +++--- fs/autofs/dev-ioctl.c |2 +- fs/autofs/inode.c | 10 +- fs/autofs/waitq.c |4 ++-- 4 files changed, 11 insertions(+), 11 deletions(-) diff --git a/fs/autofs/autofs_i.h b/fs/autofs/autofs_i.h index e99a675..fd93fdc 100644 --- a/fs/autofs/autofs_i.h +++ b/fs/autofs/autofs_i.h @@ -37,15 +37,15 @@ /* #define DEBUG */ #define DPRINTK(fmt, ...) \ - pr_debug("pid %d: %s: " fmt "\n", \ + pr_debug(KBUILD_MODNAME ":pid:%d:%s: " fmt "\n",\ current->pid, __func__, ##__VA_ARGS__) #define AUTOFS_WARN(fmt, ...) \ - pr_warn("pid %d: %s: " fmt "\n",\ + pr_warn(KBUILD_MODNAME ":pid:%d:%s: " fmt "\n", \ current->pid, __func__, ##__VA_ARGS__) #define AUTOFS_ERROR(fmt, ...) \ - pr_err("pid %d: %s: " fmt "\n", \ + pr_err(KBUILD_MODNAME ":pid:%d:%s: " fmt "\n", \ current->pid, __func__, ##__VA_ARGS__) /* diff --git a/fs/autofs/dev-ioctl.c b/fs/autofs/dev-ioctl.c index 34e515b..eb16fe4 100644 --- a/fs/autofs/dev-ioctl.c +++ b/fs/autofs/dev-ioctl.c @@ -369,7 +369,7 @@ static int autofs_dev_ioctl_setpipefd(struct file *fp, new_pid = get_task_pid(current, PIDTYPE_PGID); if (ns_of_pid(new_pid) != ns_of_pid(sbi->oz_pgrp)) { - AUTOFS_WARN("Not allowed to change PID namespace"); + AUTOFS_WARN("not allowed to change PID namespace"); err = -EINVAL; goto out; } diff --git a/fs/autofs/inode.c b/fs/autofs/inode.c index 01788cc..5012f21 100644 --- a/fs/autofs/inode.c +++ b/fs/autofs/inode.c @@ -266,14 +266,14 @@ int autofs_fill_super(struct super_block *s, void *data, int silent) if (parse_options(data, &pipefd, &root_inode->i_uid, &root_inode->i_gid, &pgrp, &pgrp_set, &sbi->type, &sbi->min_proto, &sbi->max_proto)) { - AUTOFS_ERROR("autofs: called with bogus options"); + AUTOFS_ERROR("called with bogus options"); goto fail_dput; } if (pgrp_set) { sbi->oz_pgrp = find_get_pid(pgrp); if (!sbi->oz_pgrp) { - AUTOFS_ERROR("autofs: could not find process group %d", + AUTOFS_ERROR("could not find process group %d", pgrp); goto fail_dput; } @@ -290,7 +290,7 @@ int autofs_fill_super(struct super_block *s, void *data, int silent) /* Couldn't this be tested earlier? */ if (sbi->max_proto < AUTOFS_MIN_PROTO_VERSION || sbi->min_proto > AUTOFS_MAX_PROTO_VERSION) { - AUTOFS_ERROR("autofs: kernel does not match daemon version " + AUTOFS_ERROR("kernel does not match daemon version " "daemon (%d, %d) kernel (%d, %d)", sbi->min_proto, sbi->max_proto, AUTOFS_MIN_PROTO_VERSION, AUTOFS_MAX_PROTO_VERSION); @@ -308,7 +308,7 @@ int autofs_fill_super(struct super_block *s, void *data, int silent) pipe = fget(pipefd); if (!pipe) { - AUTOFS_ERROR("autofs: could not open pipe file descriptor"); + AUTOFS_ERROR("could not open pipe file descriptor"); goto fail_dput; } ret = autofs_prepare_pipe(pipe); @@ -329,7 +329,7 @@ int autofs_fill_super(struct super_block *s, void *data, int silent) */ fail_fput: AUTOFS_ERROR( - "autofs: pipe file descriptor does not contain proper ops"); + "pipe file descriptor does not contain proper ops"); fput(pipe); /* fall through */ fail_dput: diff --git a/fs/autofs/waitq.c b/fs/autofs/waitq.c index ec0c60f..0fb2791 100644 --- a/fs/autofs/waitq.c +++ b/fs/autofs/waitq.c @@ -162,7 +162,7 @@ static void autofs_notify_daemon(struct autofs_sb_info *sbi, break; } default: - AUTOFS_WARN("autofs_notify_daemon: bad type %d!", type); + AUTOFS_WARN("bad type %d!", type); mutex_unlock(&sbi->wq_mutex); return; } @@ -450,7 +450,7 @@ int autofs_wait(struct autofs_sb_info *sbi,
Re: [PATCH 04/13] autofs4 - change printks AUTOFS defined prints
On Wed, 2014-11-05 at 15:20 -0800, Joe Perches wrote: > > > But idea of using pr_xxx() and pr_fmt() (actually that's too open to > > name clashes so it would need to be named something like autofs_pr_fmt() > > anyway) looks like it results in less readable code so I'd really prefer > > not to do that. > > Using pr_info/pr_debug (or any other pr_) is a > generic mechanism in the kernel. Adding a > #define pr_fmt is also generic thing that works with > all the pr_ uses in a specific compilation unit. LOL, so you recommend I look more closely, I will. In the meantime I'll send the other patches to Andrew. Ian -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 04/13] autofs4 - change printks AUTOFS defined prints
On Mon, 2014-11-03 at 06:33 -0800, Joe Perches wrote: > > That's fine. I left out the trailing semicolon/space. > The pr_fmt could be something like: > #define pr_fmt(fmt) KBUILD_MODNAME ":%d:%s: " fmt, current->pid, __func__ > or add a "pid:" descriptor prefix if you like too: > #define pr_fmt(fmt) KBUILD_MODNAME ":pid:%d:%s: " fmt, current->pid, __func__ > > > > it's better to use a consistent style for > > > these logging functions ideally with terminating > > > newlines so there isn't a mix of code with > > > and without those newlines. That inconsistency > > > leads to unintended defects. > > > > The idea here was to make the logging consistent throughout. > > Mine too. > > > I have become used of not using the new-line terminator in logging over > > the years and tend to favour that myself. You recommend not doing that > > probably from a kernel wide consistency perspective? Maybe that is > > better ... > > Yes, kernel style consistency is the rationale. > > Over time, people come along and add messages > while not reading the code very closely so using > the kernel style with newlines can help avoid > these trivial defects. I can see how not including the trailing newline in the macros is a good thing and I'll forward a couple more patches to Andrew for this and fix the inconsistencies. But idea of using pr_xxx() and pr_fmt() (actually that's too open to name clashes so it would need to be named something like autofs_pr_fmt() anyway) looks like it results in less readable code so I'd really prefer not to do that. But thanks for pointing out the problems with the logging calls and for you constructive suggestions. Ian -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 04/13] autofs4 - change printks AUTOFS defined prints
On Mon, 2014-11-03 at 00:25 -0800, Joe Perches wrote: > On Mon, 2014-11-03 at 16:12 +0800, Ian Kent wrote: > > Use the AUTOFS_*() print defines instead of raw printks. > > Please check the output of these conversions. > > For instance: > > > diff --git a/fs/autofs4/waitq.c b/fs/autofs4/waitq.c > [] > > @@ -162,7 +162,7 @@ static void autofs4_notify_daemon(struct autofs_sb_info > > *sbi, > > break; > > } > > default: > > - printk("autofs4_notify_daemon: bad type %d!\n", type); > > + AUTOFS_WARN("autofs4_notify_daemon: bad type %d!", type); > > mutex_unlock(&sbi->wq_mutex); > > return; > > } > > The current #define is: > > #define AUTOFS_WARN(fmt, ...) \ > pr_warn("pid %d: %s: " fmt "\n",\ > current->pid, __func__, ##__VA_ARGS__) > > So this duplicates the function name in the output. Ahh, yes, I'll fix that. > > It's probably better to simply use > #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt > or > #define pr_fmt(fmt) KBUILD_MODNAME ":%d:%s" fmt, current->pid, __func__ > if you _really_ want pid/func in the output I do, yes. > > and just use pr_ instead of AUTOFS_ macros. > > And it's better to use a consistent style for > these logging functions ideally with terminating > newlines so there isn't a mix of code with > and without those newlines. That inconsistency > leads to unintended defects. The idea here was to make the logging consistent throughout. I have become used of not using the new-line terminator in logging over the years and tend to favour that myself. You recommend not doing that probably from a kernel wide consistency perspective? Maybe that is better ... Ian -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH 03/13] autofs4 - use pr print in AUTOFS prints
Use the pr_*() print in AUTOFS_*() defines instead of printks. Signed-off-by: Ian Kent --- fs/autofs4/autofs_i.h |4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/fs/autofs4/autofs_i.h b/fs/autofs4/autofs_i.h index 0a1fcdc..b33b939 100644 --- a/fs/autofs4/autofs_i.h +++ b/fs/autofs4/autofs_i.h @@ -41,11 +41,11 @@ current->pid, __func__, ##__VA_ARGS__) #define AUTOFS_WARN(fmt, ...) \ - printk(KERN_WARNING "pid %d: %s: " fmt "\n",\ + pr_warn("pid %d: %s: " fmt "\n",\ current->pid, __func__, ##__VA_ARGS__) #define AUTOFS_ERROR(fmt, ...) \ - printk(KERN_ERR "pid %d: %s: " fmt "\n",\ + pr_err("pid %d: %s: " fmt "\n", \ current->pid, __func__, ##__VA_ARGS__) /* -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH 07/13] autofs - merge auto_fs.h and auto_fs4.h
The autofs module has long since been removed so there's no need to have two separate include files for autofs. Signed-off-by: Ian Kent --- fs/autofs4/autofs_i.h |2 - fs/compat_ioctl.c |1 include/uapi/linux/auto_fs.h | 147 ++-- include/uapi/linux/auto_fs4.h | 149 + 4 files changed, 143 insertions(+), 156 deletions(-) diff --git a/fs/autofs4/autofs_i.h b/fs/autofs4/autofs_i.h index b33b939..fc33904 100644 --- a/fs/autofs4/autofs_i.h +++ b/fs/autofs4/autofs_i.h @@ -10,7 +10,7 @@ /* Internal header file for autofs */ -#include +#include #include #include #include diff --git a/fs/compat_ioctl.c b/fs/compat_ioctl.c index afec645..8638407 100644 --- a/fs/compat_ioctl.c +++ b/fs/compat_ioctl.c @@ -38,7 +38,6 @@ #include #include #include -#include #include #include #include diff --git a/include/uapi/linux/auto_fs.h b/include/uapi/linux/auto_fs.h index 9175a1b..b3122451 100644 --- a/include/uapi/linux/auto_fs.h +++ b/include/uapi/linux/auto_fs.h @@ -1,5 +1,7 @@ /* - * Copyright 1997 Transmeta Corporation - All Rights Reserved + * Copyright 1997 Transmeta Corporation - All Rights Reserved + * Copyright 1999-2000 Jeremy Fitzhardinge + * Copyright 2005-2006,2013 Ian Kent * * This file is part of the Linux kernel and is made available under * the terms of the GNU General Public License, version 2, or at your @@ -7,7 +9,6 @@ * * --- */ - #ifndef _UAPI_LINUX_AUTO_FS_H #define _UAPI_LINUX_AUTO_FS_H @@ -16,13 +17,11 @@ #include #endif /* __KERNEL__ */ +#define AUTOFS_PROTO_VERSION 5 +#define AUTOFS_MIN_PROTO_VERSION 3 +#define AUTOFS_MAX_PROTO_VERSION 5 -/* This file describes autofs v3 */ -#define AUTOFS_PROTO_VERSION 3 - -/* Range of protocol versions defined */ -#define AUTOFS_MAX_PROTO_VERSION AUTOFS_PROTO_VERSION -#define AUTOFS_MIN_PROTO_VERSION AUTOFS_PROTO_VERSION +#define AUTOFS_PROTO_SUBVERSION2 /* * The wait_queue_token (autofs_wqt_t) is part of a structure which is passed @@ -68,4 +67,136 @@ struct autofs_packet_expire { #define AUTOFS_IOC_SETTIMEOUT _IOWR(0x93, 0x64, unsigned long) #define AUTOFS_IOC_EXPIRE _IOR(0x93, 0x65, struct autofs_packet_expire) +/* autofs version 4 and later definitions */ + +/* Mask for expire behaviour */ +#define AUTOFS_EXP_IMMEDIATE 1 +#define AUTOFS_EXP_LEAVES 2 + +#define AUTOFS_TYPE_ANY0U +#define AUTOFS_TYPE_INDIRECT 1U +#define AUTOFS_TYPE_DIRECT 2U +#define AUTOFS_TYPE_OFFSET 4U + +static inline void set_autofs_type_indirect(unsigned int *type) +{ + *type = AUTOFS_TYPE_INDIRECT; +} + +static inline unsigned int autofs_type_indirect(unsigned int type) +{ + return (type == AUTOFS_TYPE_INDIRECT); +} + +static inline void set_autofs_type_direct(unsigned int *type) +{ + *type = AUTOFS_TYPE_DIRECT; +} + +static inline unsigned int autofs_type_direct(unsigned int type) +{ + return (type == AUTOFS_TYPE_DIRECT); +} + +static inline void set_autofs_type_offset(unsigned int *type) +{ + *type = AUTOFS_TYPE_OFFSET; +} + +static inline unsigned int autofs_type_offset(unsigned int type) +{ + return (type == AUTOFS_TYPE_OFFSET); +} + +static inline unsigned int autofs_type_trigger(unsigned int type) +{ + return (type == AUTOFS_TYPE_DIRECT || type == AUTOFS_TYPE_OFFSET); +} + +/* + * This isn't really a type as we use it to say "no type set" to + * indicate we want to search for "any" mount in the + * autofs_dev_ioctl_ismountpoint() device ioctl function. + */ +static inline void set_autofs_type_any(unsigned int *type) +{ + *type = AUTOFS_TYPE_ANY; +} + +static inline unsigned int autofs_type_any(unsigned int type) +{ + return (type == AUTOFS_TYPE_ANY); +} + +/* Daemon notification packet types */ +enum autofs_notify { + NFY_NONE, + NFY_MOUNT, + NFY_EXPIRE +}; + +/* Kernel protocol version 4 packet types */ + +/* Expire entry (umount request) */ +#define autofs_ptype_expire_multi 2 + +/* Kernel protocol version 5 packet types */ + +/* Indirect mount missing and expire requests. */ +#define autofs_ptype_missing_indirect 3 +#define autofs_ptype_expire_indirect 4 + +/* Direct mount missing and expire requests */ +#define autofs_ptype_missing_direct5 +#define autofs_ptype_expire_direct 6 + +/* v4 multi expire (via pipe) */ +struct autofs_packet_expire_multi { + struct autofs_packet_hdr hdr; + autofs_wqt_t wait_queue_token; + int len; + char name[NAME_MAX+1]; +}; + +union autofs_packet_union { + struct autofs_packet_hdr hdr; + struct autofs_packet_missing missing; + struct autofs_packet_expire expire; + struct autofs_packet_expire_multi expire_multi; +};
[PATCH 11/13] autofs - update fs/autofs4/Kconfig
Update Kconfig and add a depricated warning. Signed-off-by: Ian Kent --- fs/autofs4/Kconfig | 32 ++-- 1 file changed, 22 insertions(+), 10 deletions(-) diff --git a/fs/autofs4/Kconfig b/fs/autofs4/Kconfig index 1204d63..26dbb00 100644 --- a/fs/autofs4/Kconfig +++ b/fs/autofs4/Kconfig @@ -1,5 +1,6 @@ config AUTOFS4_FS - tristate "Kernel automounter version 4 support (also supports v3)" + tristate "Kernel automounter support (supports v3, v4 and v5)" + depends on AUTOFS_FS = n help The automounter is a tool to automatically mount remote file systems on demand. This implementation is partially kernel-based to reduce @@ -7,14 +8,25 @@ config AUTOFS4_FS automounter (amd), which is a pure user space daemon. To use the automounter you need the user-space tools from - <ftp://ftp.kernel.org/pub/linux/daemons/autofs/v4/>; you also - want to answer Y to "NFS file system support", below. + <ftp://ftp.kernel.org/pub/linux/daemons/autofs/>; you also want + to answer Y to "NFS file system support", below. - To compile this support as a module, choose M here: the module will be - called autofs4. You will need to add "alias autofs autofs4" to your - modules configuration file. + This module is in the process of being renamed from autofs4 to + autofs. Since autofs is now the only module that provides the + autofs file system the module is not version 4 specific. - If you are not a part of a fairly large, distributed network or - don't have a laptop which needs to dynamically reconfigure to the - local network, you probably do not need an automounter, and can say - N here. + The autofs4 module is now built from the source located in + fs/autofs. The autofs4 directory and its configuration entry + will be removed two kernel version from the inclusion of this + change. + + Changes that will need to be made should be limited to: + - source include statments should be changed from autofs_fs4.h to + autofs_fs.h since these two header files have been merged. + - user space scripts that manually load autofs4.ko should be + changed to load autofs.ko. But since the module directory name + and the module name are the same as the file system name there + is no need to manually load module. + - any "alias autofs autofs4" will need to be removed. + + Please configure AUTOFS_FS instead of AUTOFS4_FS from now on. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH 09/13] autofs - copy autofs4 to autofs
Copy source files from the autofs4 directory to the autofs directory. Signed-off-by: Ian Kent --- fs/autofs/autofs_i.h | 283 +++ fs/autofs/dev-ioctl.c | 763 fs/autofs/expire.c| 618 fs/autofs/init.c | 49 +++ fs/autofs/inode.c | 371 +++ fs/autofs/root.c | 939 + fs/autofs/symlink.c | 26 + fs/autofs/waitq.c | 567 ++ 8 files changed, 3616 insertions(+) create mode 100644 fs/autofs/autofs_i.h create mode 100644 fs/autofs/dev-ioctl.c create mode 100644 fs/autofs/expire.c create mode 100644 fs/autofs/init.c create mode 100644 fs/autofs/inode.c create mode 100644 fs/autofs/root.c create mode 100644 fs/autofs/symlink.c create mode 100644 fs/autofs/waitq.c diff --git a/fs/autofs/autofs_i.h b/fs/autofs/autofs_i.h new file mode 100644 index 000..e99a675 --- /dev/null +++ b/fs/autofs/autofs_i.h @@ -0,0 +1,283 @@ +/* + * Copyright 1997-1998 Transmeta Corporation - All Rights Reserved + * Copyright 2005-2006 Ian Kent + * + * This file is part of the Linux kernel and is made available under + * the terms of the GNU General Public License, version 2, or at your + * option, any later version, incorporated herein by reference. + * + * --- */ + +/* Internal header file for autofs */ + +#include +#include +#include +#include +#include + +/* This is the range of ioctl() numbers we claim as ours */ +#define AUTOFS_IOC_FIRST AUTOFS_IOC_READY +#define AUTOFS_IOC_COUNT 32 + +#define AUTOFS_DEV_IOCTL_IOC_FIRST (AUTOFS_DEV_IOCTL_VERSION) +#define AUTOFS_DEV_IOCTL_IOC_COUNT (AUTOFS_IOC_COUNT - 11) + +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +/* #define DEBUG */ + +#define DPRINTK(fmt, ...) \ + pr_debug("pid %d: %s: " fmt "\n", \ + current->pid, __func__, ##__VA_ARGS__) + +#define AUTOFS_WARN(fmt, ...) \ + pr_warn("pid %d: %s: " fmt "\n",\ + current->pid, __func__, ##__VA_ARGS__) + +#define AUTOFS_ERROR(fmt, ...) \ + pr_err("pid %d: %s: " fmt "\n", \ + current->pid, __func__, ##__VA_ARGS__) + +/* + * Unified info structure. This is pointed to by both the dentry and + * inode structures. Each file in the filesystem has an instance of this + * structure. It holds a reference to the dentry, so dentries are never + * flushed while the file exists. All name lookups are dealt with at the + * dentry level, although the filesystem can interfere in the validation + * process. Readdir is implemented by traversing the dentry lists. + */ +struct autofs_info { + struct dentry *dentry; + struct inode*inode; + + int flags; + + struct completion expire_complete; + + struct list_head active; + int active_count; + + struct list_head expiring; + + struct autofs_sb_info *sbi; + unsigned long last_used; + atomic_t count; + + kuid_t uid; + kgid_t gid; +}; + +#define AUTOFS_INF_EXPIRING(1<<0) /* dentry in the process of expiring */ +#define AUTOFS_INF_NO_RCU (1<<1) /* the dentry is being considered + * for expiry, so RCU_walk is + * not permitted + */ +#define AUTOFS_INF_PENDING (1<<2) /* dentry pending mount */ + +struct autofs_wait_queue { + wait_queue_head_t queue; + struct autofs_wait_queue *next; + autofs_wqt_t wait_queue_token; + /* We use the following to see what we are waiting for */ + struct qstr name; + u32 dev; + u64 ino; + kuid_t uid; + kgid_t gid; + pid_t pid; + pid_t tgid; + /* This is for status reporting upon return */ + int status; + unsigned int wait_ctr; +}; + +#define AUTOFS_SBI_MAGIC 0x6d4a556d + +struct autofs_sb_info { + u32 magic; + int pipefd; + struct file *pipe; + struct pid *oz_pgrp; + int catatonic; + int version; + int sub_version; + int min_proto; + int max_proto; + unsigned long exp_timeout; + unsigned int type; + int reghost_enabled; + int needs_reghost; + struct super_block *sb; + struct mutex wq_mutex; + struct mutex pipe_mutex; + spinlock_t fs_lock; + struct autofs_wait_queue *queues; /* Wait queue pointer */ + spinlock_t lookup_lock; + struct list_head active_list; + struct list_head expiring_list; + struct rcu_head rcu; +}; + +static inline struct auto
[PATCH 13/13] autofs - delete fs/autofs4 source files
Delete the now unused autofs4 module files. Signed-off-by: Ian Kent --- fs/autofs4/autofs_i.h | 283 -- fs/autofs4/dev-ioctl.c | 764 --- fs/autofs4/expire.c| 619 fs/autofs4/init.c | 49 --- fs/autofs4/inode.c | 371 --- fs/autofs4/root.c | 939 fs/autofs4/symlink.c | 26 - fs/autofs4/waitq.c | 568 - 8 files changed, 3619 deletions(-) delete mode 100644 fs/autofs4/autofs_i.h delete mode 100644 fs/autofs4/dev-ioctl.c delete mode 100644 fs/autofs4/expire.c delete mode 100644 fs/autofs4/init.c delete mode 100644 fs/autofs4/inode.c delete mode 100644 fs/autofs4/root.c delete mode 100644 fs/autofs4/symlink.c delete mode 100644 fs/autofs4/waitq.c diff --git a/fs/autofs4/autofs_i.h b/fs/autofs4/autofs_i.h deleted file mode 100644 index e99a675..000 --- a/fs/autofs4/autofs_i.h +++ /dev/null @@ -1,283 +0,0 @@ -/* - * Copyright 1997-1998 Transmeta Corporation - All Rights Reserved - * Copyright 2005-2006 Ian Kent - * - * This file is part of the Linux kernel and is made available under - * the terms of the GNU General Public License, version 2, or at your - * option, any later version, incorporated herein by reference. - * - * --- */ - -/* Internal header file for autofs */ - -#include -#include -#include -#include -#include - -/* This is the range of ioctl() numbers we claim as ours */ -#define AUTOFS_IOC_FIRST AUTOFS_IOC_READY -#define AUTOFS_IOC_COUNT 32 - -#define AUTOFS_DEV_IOCTL_IOC_FIRST (AUTOFS_DEV_IOCTL_VERSION) -#define AUTOFS_DEV_IOCTL_IOC_COUNT (AUTOFS_IOC_COUNT - 11) - -#include -#include -#include -#include -#include -#include -#include -#include -#include -#include - -/* #define DEBUG */ - -#define DPRINTK(fmt, ...) \ - pr_debug("pid %d: %s: " fmt "\n", \ - current->pid, __func__, ##__VA_ARGS__) - -#define AUTOFS_WARN(fmt, ...) \ - pr_warn("pid %d: %s: " fmt "\n",\ - current->pid, __func__, ##__VA_ARGS__) - -#define AUTOFS_ERROR(fmt, ...) \ - pr_err("pid %d: %s: " fmt "\n", \ - current->pid, __func__, ##__VA_ARGS__) - -/* - * Unified info structure. This is pointed to by both the dentry and - * inode structures. Each file in the filesystem has an instance of this - * structure. It holds a reference to the dentry, so dentries are never - * flushed while the file exists. All name lookups are dealt with at the - * dentry level, although the filesystem can interfere in the validation - * process. Readdir is implemented by traversing the dentry lists. - */ -struct autofs_info { - struct dentry *dentry; - struct inode*inode; - - int flags; - - struct completion expire_complete; - - struct list_head active; - int active_count; - - struct list_head expiring; - - struct autofs_sb_info *sbi; - unsigned long last_used; - atomic_t count; - - kuid_t uid; - kgid_t gid; -}; - -#define AUTOFS_INF_EXPIRING(1<<0) /* dentry in the process of expiring */ -#define AUTOFS_INF_NO_RCU (1<<1) /* the dentry is being considered - * for expiry, so RCU_walk is - * not permitted - */ -#define AUTOFS_INF_PENDING (1<<2) /* dentry pending mount */ - -struct autofs_wait_queue { - wait_queue_head_t queue; - struct autofs_wait_queue *next; - autofs_wqt_t wait_queue_token; - /* We use the following to see what we are waiting for */ - struct qstr name; - u32 dev; - u64 ino; - kuid_t uid; - kgid_t gid; - pid_t pid; - pid_t tgid; - /* This is for status reporting upon return */ - int status; - unsigned int wait_ctr; -}; - -#define AUTOFS_SBI_MAGIC 0x6d4a556d - -struct autofs_sb_info { - u32 magic; - int pipefd; - struct file *pipe; - struct pid *oz_pgrp; - int catatonic; - int version; - int sub_version; - int min_proto; - int max_proto; - unsigned long exp_timeout; - unsigned int type; - int reghost_enabled; - int needs_reghost; - struct super_block *sb; - struct mutex wq_mutex; - struct mutex pipe_mutex; - spinlock_t fs_lock; - struct autofs_wait_queue *queues; /* Wait queue pointer */ - spinlock_t lookup_lock; - struct list_head active_list; - struct list_head expiring_list; - struct rcu_head rcu; -}; - -static inline struct autofs_sb_
[PATCH 06/13] autofs4 - move linux/auto_dev-ioctl.h to uapi/linux
Since linux/auto_dev-ioctl.h wasn't included in include/linux/Kbuild (although I should have added it) it wasn't moved to uapi/linux as part of the uapi series. Signed-off-by: Ian Kent --- include/linux/auto_dev-ioctl.h | 223 --- include/uapi/linux/auto_dev-ioctl.h | 223 +++ 2 files changed, 223 insertions(+), 223 deletions(-) delete mode 100644 include/linux/auto_dev-ioctl.h create mode 100644 include/uapi/linux/auto_dev-ioctl.h diff --git a/include/linux/auto_dev-ioctl.h b/include/linux/auto_dev-ioctl.h deleted file mode 100644 index 7caaf29..000 --- a/include/linux/auto_dev-ioctl.h +++ /dev/null @@ -1,223 +0,0 @@ -/* - * Copyright 2008 Red Hat, Inc. All rights reserved. - * Copyright 2008 Ian Kent - * - * This file is part of the Linux kernel and is made available under - * the terms of the GNU General Public License, version 2, or at your - * option, any later version, incorporated herein by reference. - */ - -#ifndef _LINUX_AUTO_DEV_IOCTL_H -#define _LINUX_AUTO_DEV_IOCTL_H - -#include -#include - -#define AUTOFS_DEVICE_NAME "autofs" - -#define AUTOFS_DEV_IOCTL_VERSION_MAJOR 1 -#define AUTOFS_DEV_IOCTL_VERSION_MINOR 0 - -#define AUTOFS_DEVID_LEN 16 - -#define AUTOFS_DEV_IOCTL_SIZE sizeof(struct autofs_dev_ioctl) - -/* - * An ioctl interface for autofs mount point control. - */ - -struct args_protover { - __u32 version; -}; - -struct args_protosubver { - __u32 sub_version; -}; - -struct args_openmount { - __u32 devid; -}; - -struct args_ready { - __u32 token; -}; - -struct args_fail { - __u32 token; - __s32 status; -}; - -struct args_setpipefd { - __s32 pipefd; -}; - -struct args_timeout { - __u64 timeout; -}; - -struct args_requester { - __u32 uid; - __u32 gid; -}; - -struct args_expire { - __u32 how; -}; - -struct args_askumount { - __u32 may_umount; -}; - -struct args_ismountpoint { - union { - struct args_in { - __u32 type; - } in; - struct args_out { - __u32 devid; - __u32 magic; - } out; - }; -}; - -/* - * All the ioctls use this structure. - * When sending a path size must account for the total length - * of the chunk of memory otherwise is is the size of the - * structure. - */ - -struct autofs_dev_ioctl { - __u32 ver_major; - __u32 ver_minor; - __u32 size; /* total size of data passed in -* including this struct */ - __s32 ioctlfd; /* automount command fd */ - - /* Command parameters */ - - union { - struct args_protoverprotover; - struct args_protosubver protosubver; - struct args_openmount openmount; - struct args_ready ready; - struct args_failfail; - struct args_setpipefd setpipefd; - struct args_timeout timeout; - struct args_requester requester; - struct args_expire expire; - struct args_askumount askumount; - struct args_ismountpointismountpoint; - }; - - char path[0]; -}; - -static inline void init_autofs_dev_ioctl(struct autofs_dev_ioctl *in) -{ - memset(in, 0, sizeof(struct autofs_dev_ioctl)); - in->ver_major = AUTOFS_DEV_IOCTL_VERSION_MAJOR; - in->ver_minor = AUTOFS_DEV_IOCTL_VERSION_MINOR; - in->size = sizeof(struct autofs_dev_ioctl); - in->ioctlfd = -1; -} - -/* - * If you change this make sure you make the corresponding change - * to autofs-dev-ioctl.c:lookup_ioctl() - */ -enum { - /* Get various version info */ - AUTOFS_DEV_IOCTL_VERSION_CMD = 0x71, - AUTOFS_DEV_IOCTL_PROTOVER_CMD, - AUTOFS_DEV_IOCTL_PROTOSUBVER_CMD, - - /* Open mount ioctl fd */ - AUTOFS_DEV_IOCTL_OPENMOUNT_CMD, - - /* Close mount ioctl fd */ - AUTOFS_DEV_IOCTL_CLOSEMOUNT_CMD, - - /* Mount/expire status returns */ - AUTOFS_DEV_IOCTL_READY_CMD, - AUTOFS_DEV_IOCTL_FAIL_CMD, - - /* Activate/deactivate autofs mount */ - AUTOFS_DEV_IOCTL_SETPIPEFD_CMD, - AUTOFS_DEV_IOCTL_CATATONIC_CMD, - - /* Expiry timeout */ - AUTOFS_DEV_IOCTL_TIMEOUT_CMD, - - /* Get mount last requesting uid and gid */ - AUTOFS_DEV_IOCTL_REQUESTER_CMD, - - /* Check for eligible expire candidates */ - AUTOFS_DEV_IOCTL_EXPIRE_CMD, - - /* Request busy status */ - AUTOFS_DEV_IOCTL_ASKUMOUNT_CMD, - - /* Check if path is a mountpoint */ - AUTOFS_DEV_IOCTL_ISMOUNTPOINT_CMD, -}; - -#define AUTOFS_IOCTL 0x93 - -#define AUTOFS_D
[PATCH 10/13] autofs - create autofs Kconfig and Makefile
Create Makefile and Kconfig for autofs module. Signed-off-by: Ian Kent --- fs/Kconfig |1 + fs/Makefile|1 + fs/autofs/Kconfig | 19 +++ fs/autofs/Makefile |7 +++ 4 files changed, 28 insertions(+) create mode 100644 fs/autofs/Kconfig create mode 100644 fs/autofs/Makefile diff --git a/fs/Kconfig b/fs/Kconfig index 664991a..18fa31d 100644 --- a/fs/Kconfig +++ b/fs/Kconfig @@ -65,6 +65,7 @@ source "fs/notify/Kconfig" source "fs/quota/Kconfig" +source "fs/autofs/Kconfig" source "fs/autofs4/Kconfig" source "fs/fuse/Kconfig" source "fs/overlayfs/Kconfig" diff --git a/fs/Makefile b/fs/Makefile index 34a1b9de..7e1b396 100644 --- a/fs/Makefile +++ b/fs/Makefile @@ -101,6 +101,7 @@ obj-$(CONFIG_AFFS_FS) += affs/ obj-$(CONFIG_ROMFS_FS) += romfs/ obj-$(CONFIG_QNX4FS_FS)+= qnx4/ obj-$(CONFIG_QNX6FS_FS)+= qnx6/ +obj-$(CONFIG_AUTOFS_FS)+= autofs/ obj-$(CONFIG_AUTOFS4_FS) += autofs4/ obj-$(CONFIG_ADFS_FS) += adfs/ obj-$(CONFIG_FUSE_FS) += fuse/ diff --git a/fs/autofs/Kconfig b/fs/autofs/Kconfig new file mode 100644 index 000..17f77a6 --- /dev/null +++ b/fs/autofs/Kconfig @@ -0,0 +1,19 @@ +config AUTOFS_FS + tristate "Kernel automounter support (supports v3, v4 and v5)" + help + The automounter is a tool to automatically mount remote file systems + on demand. This implementation is partially kernel-based to reduce + overhead in the already-mounted case; this is unlike the BSD + automounter (amd), which is a pure user space daemon. + + To use the automounter you need the user-space tools from + <ftp://ftp.kernel.org/pub/linux/daemons/autofs/>; you also want + to answer Y to "NFS file system support", below. + + To compile this support as a module, choose M here: the module will be + called autofs. + + If you are not a part of a fairly large, distributed network or + don't have a laptop which needs to dynamically reconfigure to the + local network, you probably do not need an automounter, and can say + N here. diff --git a/fs/autofs/Makefile b/fs/autofs/Makefile new file mode 100644 index 000..43fedde --- /dev/null +++ b/fs/autofs/Makefile @@ -0,0 +1,7 @@ +# +# Makefile for the linux autofs-filesystem routines. +# + +obj-$(CONFIG_AUTOFS_FS) += autofs.o + +autofs-objs := init.o inode.o root.o symlink.o waitq.o expire.o dev-ioctl.o -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH 05/13] autofs4 - fix string.h include in auto_dev-ioctl.h
Since including linux/string.h will now do the right thing remove the conditional check. Signed-off-by: Ian Kent --- include/linux/auto_dev-ioctl.h |5 - 1 file changed, 5 deletions(-) diff --git a/include/linux/auto_dev-ioctl.h b/include/linux/auto_dev-ioctl.h index 6427816..7caaf29 100644 --- a/include/linux/auto_dev-ioctl.h +++ b/include/linux/auto_dev-ioctl.h @@ -11,12 +11,7 @@ #define _LINUX_AUTO_DEV_IOCTL_H #include - -#ifdef __KERNEL__ #include -#else -#include -#endif /* __KERNEL__ */ #define AUTOFS_DEVICE_NAME "autofs" -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH 12/13] autofs - update fs/autofs4/Makefile
Update Makefile to build from source in fs/autofs instead of fs/autofs4. Signed-off-by: Ian Kent --- fs/autofs4/Makefile |4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/fs/autofs4/Makefile b/fs/autofs4/Makefile index a811c1f..417dd72 100644 --- a/fs/autofs4/Makefile +++ b/fs/autofs4/Makefile @@ -4,4 +4,6 @@ obj-$(CONFIG_AUTOFS4_FS) += autofs4.o -autofs4-objs := init.o inode.o root.o symlink.o waitq.o expire.o dev-ioctl.o +autofs4-objs := ../autofs/init.o ../autofs/inode.o ../autofs/root.o \ + ../autofs/symlink.o ../autofs/waitq.o ../autofs/expire.o \ + ../autofs/dev-ioctl.o -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH 08/13] autofs - use autofs instead of autofs4 everywhere
Update naming within autofs source to be consistent by changing occurrences of autofs4 to autofs. Signed-off-by: Ian Kent --- fs/autofs4/autofs_i.h | 88 fs/autofs4/dev-ioctl.c | 18 ++- fs/autofs4/expire.c| 132 fs/autofs4/init.c | 12 +- fs/autofs4/inode.c | 46 fs/autofs4/root.c | 269 fs/autofs4/symlink.c | 12 +- fs/autofs4/waitq.c | 55 +- 8 files changed, 316 insertions(+), 316 deletions(-) diff --git a/fs/autofs4/autofs_i.h b/fs/autofs4/autofs_i.h index fc33904..e99a675 100644 --- a/fs/autofs4/autofs_i.h +++ b/fs/autofs4/autofs_i.h @@ -128,44 +128,44 @@ struct autofs_sb_info { struct rcu_head rcu; }; -static inline struct autofs_sb_info *autofs4_sbi(struct super_block *sb) +static inline struct autofs_sb_info *autofs_sbi(struct super_block *sb) { return (struct autofs_sb_info *)(sb->s_fs_info); } -static inline struct autofs_info *autofs4_dentry_ino(struct dentry *dentry) +static inline struct autofs_info *autofs_dentry_ino(struct dentry *dentry) { return (struct autofs_info *)(dentry->d_fsdata); } -/* autofs4_oz_mode(): do we see the man behind the curtain? (The +/* autofs_oz_mode(): do we see the man behind the curtain? (The processes which do manipulations for us in user space sees the raw filesystem without "magic".) */ -static inline int autofs4_oz_mode(struct autofs_sb_info *sbi) +static inline int autofs_oz_mode(struct autofs_sb_info *sbi) { return sbi->catatonic || task_pgrp(current) == sbi->oz_pgrp; } -struct inode *autofs4_get_inode(struct super_block *, umode_t); -void autofs4_free_ino(struct autofs_info *); +struct inode *autofs_get_inode(struct super_block *, umode_t); +void autofs_free_ino(struct autofs_info *); /* Expiration */ -int is_autofs4_dentry(struct dentry *); -int autofs4_expire_wait(struct dentry *dentry, int rcu_walk); -int autofs4_expire_run(struct super_block *, struct vfsmount *, - struct autofs_sb_info *, - struct autofs_packet_expire __user *); -int autofs4_do_expire_multi(struct super_block *sb, struct vfsmount *mnt, - struct autofs_sb_info *sbi, int when); -int autofs4_expire_multi(struct super_block *, struct vfsmount *, -struct autofs_sb_info *, int __user *); -struct dentry *autofs4_expire_direct(struct super_block *sb, -struct vfsmount *mnt, -struct autofs_sb_info *sbi, int how); -struct dentry *autofs4_expire_indirect(struct super_block *sb, - struct vfsmount *mnt, - struct autofs_sb_info *sbi, int how); +int is_autofs_dentry(struct dentry *); +int autofs_expire_wait(struct dentry *dentry, int rcu_walk); +int autofs_expire_run(struct super_block *, struct vfsmount *, + struct autofs_sb_info *, + struct autofs_packet_expire __user *); +int autofs_do_expire_multi(struct super_block *sb, struct vfsmount *mnt, + struct autofs_sb_info *sbi, int when); +int autofs_expire_multi(struct super_block *, struct vfsmount *, + struct autofs_sb_info *, int __user *); +struct dentry *autofs_expire_direct(struct super_block *sb, + struct vfsmount *mnt, + struct autofs_sb_info *sbi, int how); +struct dentry *autofs_expire_indirect(struct super_block *sb, + struct vfsmount *mnt, + struct autofs_sb_info *sbi, int how); /* Device node initialization */ @@ -174,11 +174,11 @@ void autofs_dev_ioctl_exit(void); /* Operations structures */ -extern const struct inode_operations autofs4_symlink_inode_operations; -extern const struct inode_operations autofs4_dir_inode_operations; -extern const struct file_operations autofs4_dir_operations; -extern const struct file_operations autofs4_root_operations; -extern const struct dentry_operations autofs4_dentry_operations; +extern const struct inode_operations autofs_symlink_inode_operations; +extern const struct inode_operations autofs_dir_inode_operations; +extern const struct file_operations autofs_dir_operations; +extern const struct file_operations autofs_root_operations; +extern const struct dentry_operations autofs_dentry_operations; /* VFS automount flags management functions */ static inline void __managed_dentry_set_managed(struct dentry *dentry) @@ -207,9 +207,9 @@ static inline void managed_dentry_clear_managed(struct dentry *dentry) /* Initializing function */ -int autofs4_fill_super(struct super_block *, void *, int); -struct autofs_info *autofs4_new_ino(struct autofs_sb_info *); -void autofs4_clean_ino(struct autofs_info *);
[PATCH 02/13] autofs4 - fix some white space errors
Fix some white space format errors. Signed-off-by: Ian Kent --- fs/autofs4/inode.c|2 +- fs/autofs4/root.c |8 fs/autofs4/waitq.c|2 +- include/uapi/linux/auto_fs.h |2 +- include/uapi/linux/auto_fs4.h |2 +- 5 files changed, 8 insertions(+), 8 deletions(-) diff --git a/fs/autofs4/inode.c b/fs/autofs4/inode.c index e855334..80389af 100644 --- a/fs/autofs4/inode.c +++ b/fs/autofs4/inode.c @@ -323,7 +323,7 @@ int autofs4_fill_super(struct super_block *s, void *data, int silent) */ s->s_root = root; return 0; - + /* * Failure ... clean up. */ diff --git a/fs/autofs4/root.c b/fs/autofs4/root.c index 373be98..7f3e182 100644 --- a/fs/autofs4/root.c +++ b/fs/autofs4/root.c @@ -624,7 +624,7 @@ static int autofs4_dir_unlink(struct inode *dir, struct dentry *dentry) struct autofs_sb_info *sbi = autofs4_sbi(dir->i_sb); struct autofs_info *ino = autofs4_dentry_ino(dentry); struct autofs_info *p_ino; - + /* This allows root to remove symlinks */ if (!autofs4_oz_mode(sbi) && !capable(CAP_SYS_ADMIN)) return -EPERM; @@ -704,7 +704,7 @@ static int autofs4_dir_rmdir(struct inode *dir, struct dentry *dentry) struct autofs_sb_info *sbi = autofs4_sbi(dir->i_sb); struct autofs_info *ino = autofs4_dentry_ino(dentry); struct autofs_info *p_ino; - + DPRINTK("dentry %p, removing %.*s", dentry, dentry->d_name.len, dentry->d_name.name); @@ -874,10 +874,10 @@ static int autofs4_root_ioctl_unlocked(struct inode *inode, struct file *filp, if (_IOC_TYPE(cmd) != _IOC_TYPE(AUTOFS_IOC_FIRST) || _IOC_NR(cmd) - _IOC_NR(AUTOFS_IOC_FIRST) >= AUTOFS_IOC_COUNT) return -ENOTTY; - + if (!autofs4_oz_mode(sbi) && !capable(CAP_SYS_ADMIN)) return -EPERM; - + switch (cmd) { case AUTOFS_IOC_READY: /* Wait queue: go ahead and retry */ return autofs4_wait_release(sbi, (autofs_wqt_t) arg, 0); diff --git a/fs/autofs4/waitq.c b/fs/autofs4/waitq.c index a0b6a97..b7f2deb 100644 --- a/fs/autofs4/waitq.c +++ b/fs/autofs4/waitq.c @@ -86,7 +86,7 @@ static int autofs4_write(struct autofs_sb_info *sbi, return (bytes > 0); } - + static void autofs4_notify_daemon(struct autofs_sb_info *sbi, struct autofs_wait_queue *wq, int type) diff --git a/include/uapi/linux/auto_fs.h b/include/uapi/linux/auto_fs.h index 5fe176a..9175a1b 100644 --- a/include/uapi/linux/auto_fs.h +++ b/include/uapi/linux/auto_fs.h @@ -48,7 +48,7 @@ struct autofs_packet_hdr { struct autofs_packet_missing { struct autofs_packet_hdr hdr; -autofs_wqt_t wait_queue_token; + autofs_wqt_t wait_queue_token; int len; char name[NAME_MAX+1]; }; diff --git a/include/uapi/linux/auto_fs4.h b/include/uapi/linux/auto_fs4.h index 924fb1a..8f8f1bd 100644 --- a/include/uapi/linux/auto_fs4.h +++ b/include/uapi/linux/auto_fs4.h @@ -108,7 +108,7 @@ enum autofs_notify { /* v4 multi expire (via pipe) */ struct autofs_packet_expire_multi { struct autofs_packet_hdr hdr; -autofs_wqt_t wait_queue_token; + autofs_wqt_t wait_queue_token; int len; char name[NAME_MAX+1]; }; -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH 01/13] autofs4 - coding style fixes
Try and make the coding style completely consistent throughtout the autofs module and inline with kernel coding style recommendations. Signed-off-by: Ian Kent --- fs/autofs4/autofs_i.h | 42 +++-- fs/autofs4/dev-ioctl.c | 24 fs/autofs4/expire.c| 54 +++ fs/autofs4/init.c |5 +-- fs/autofs4/inode.c | 18 + fs/autofs4/root.c | 80 +++- fs/autofs4/symlink.c |6 +-- fs/autofs4/waitq.c | 30 --- include/linux/auto_dev-ioctl.h |1 - include/linux/auto_fs.h|5 +-- include/uapi/linux/auto_fs.h | 19 -- include/uapi/linux/auto_fs4.h | 15 ++-- 12 files changed, 152 insertions(+), 147 deletions(-) diff --git a/fs/autofs4/autofs_i.h b/fs/autofs4/autofs_i.h index 8e98cf9..0a1fcdc 100644 --- a/fs/autofs4/autofs_i.h +++ b/fs/autofs4/autofs_i.h @@ -1,7 +1,4 @@ -/* -*- c -*- - * - * - * linux/fs/autofs/autofs_i.h - * +/* * Copyright 1997-1998 Transmeta Corporation - All Rights Reserved * Copyright 2005-2006 Ian Kent * @@ -35,7 +32,7 @@ #include #include #include -#include +#include /* #define DEBUG */ @@ -51,12 +48,14 @@ printk(KERN_ERR "pid %d: %s: " fmt "\n",\ current->pid, __func__, ##__VA_ARGS__) -/* Unified info structure. This is pointed to by both the dentry and - inode structures. Each file in the filesystem has an instance of this - structure. It holds a reference to the dentry, so dentries are never - flushed while the file exists. All name lookups are dealt with at the - dentry level, although the filesystem can interfere in the validation - process. Readdir is implemented by traversing the dentry lists. */ +/* + * Unified info structure. This is pointed to by both the dentry and + * inode structures. Each file in the filesystem has an instance of this + * structure. It holds a reference to the dentry, so dentries are never + * flushed while the file exists. All name lookups are dealt with at the + * dentry level, although the filesystem can interfere in the validation + * process. Readdir is implemented by traversing the dentry lists. + */ struct autofs_info { struct dentry *dentry; struct inode*inode; @@ -78,7 +77,7 @@ struct autofs_info { kgid_t gid; }; -#define AUTOFS_INF_EXPIRING(1<<0) /* dentry is in the process of expiring */ +#define AUTOFS_INF_EXPIRING(1<<0) /* dentry in the process of expiring */ #define AUTOFS_INF_NO_RCU (1<<1) /* the dentry is being considered * for expiry, so RCU_walk is * not permitted @@ -143,7 +142,8 @@ static inline struct autofs_info *autofs4_dentry_ino(struct dentry *dentry) processes which do manipulations for us in user space sees the raw filesystem without "magic".) */ -static inline int autofs4_oz_mode(struct autofs_sb_info *sbi) { +static inline int autofs4_oz_mode(struct autofs_sb_info *sbi) +{ return sbi->catatonic || task_pgrp(current) == sbi->oz_pgrp; } @@ -154,12 +154,12 @@ void autofs4_free_ino(struct autofs_info *); int is_autofs4_dentry(struct dentry *); int autofs4_expire_wait(struct dentry *dentry, int rcu_walk); int autofs4_expire_run(struct super_block *, struct vfsmount *, - struct autofs_sb_info *, - struct autofs_packet_expire __user *); + struct autofs_sb_info *, + struct autofs_packet_expire __user *); int autofs4_do_expire_multi(struct super_block *sb, struct vfsmount *mnt, struct autofs_sb_info *sbi, int when); int autofs4_expire_multi(struct super_block *, struct vfsmount *, - struct autofs_sb_info *, int __user *); +struct autofs_sb_info *, int __user *); struct dentry *autofs4_expire_direct(struct super_block *sb, struct vfsmount *mnt, struct autofs_sb_info *sbi, int how); @@ -224,8 +224,8 @@ static inline int autofs_prepare_pipe(struct file *pipe) /* Queue management functions */ -int autofs4_wait(struct autofs_sb_info *,struct dentry *, enum autofs_notify); -int autofs4_wait_release(struct autofs_sb_info *,autofs_wqt_t,int); +int autofs4_wait(struct autofs_sb_info *, struct dentry *, enum autofs_notify); +int autofs4_wait_release(struct autofs_sb_info *, autofs_wqt_t, int); void autofs4_catatonic_mode(struct autofs_sb_info *); static inline u32 autofs4_get_dev(struct autofs_sb_info *sbi) @@ -247,37 +247,37 @@ static inline void __autofs4_add_expiring(struct dentry *dentry) { struct autofs