Commit e0e81739 authored by David Howells's avatar David Howells Committed by James Morris

CRED: Add some configurable debugging [try #6]

Add a config option (CONFIG_DEBUG_CREDENTIALS) to turn on some debug checking
for credential management.  The additional code keeps track of the number of
pointers from task_structs to any given cred struct, and checks to see that
this number never exceeds the usage count of the cred struct (which includes
all references, not just those from task_structs).

Furthermore, if SELinux is enabled, the code also checks that the security
pointer in the cred struct is never seen to be invalid.

This attempts to catch the bug whereby inode_has_perm() faults in an nfsd
kernel thread on seeing cred->security be a NULL pointer (it appears that the
credential struct has been previously released):

	http://www.kerneloops.org/oops.php?number=252883Signed-off-by: default avatarDavid Howells <dhowells@redhat.com>
Signed-off-by: default avatarJames Morris <jmorris@namei.org>
parent ed6d76e4
...@@ -34,6 +34,8 @@ int nfsd_setuser(struct svc_rqst *rqstp, struct svc_export *exp) ...@@ -34,6 +34,8 @@ int nfsd_setuser(struct svc_rqst *rqstp, struct svc_export *exp)
int flags = nfsexp_flags(rqstp, exp); int flags = nfsexp_flags(rqstp, exp);
int ret; int ret;
validate_process_creds();
/* discard any old override before preparing the new set */ /* discard any old override before preparing the new set */
revert_creds(get_cred(current->real_cred)); revert_creds(get_cred(current->real_cred));
new = prepare_creds(); new = prepare_creds();
...@@ -86,8 +88,10 @@ int nfsd_setuser(struct svc_rqst *rqstp, struct svc_export *exp) ...@@ -86,8 +88,10 @@ int nfsd_setuser(struct svc_rqst *rqstp, struct svc_export *exp)
else else
new->cap_effective = cap_raise_nfsd_set(new->cap_effective, new->cap_effective = cap_raise_nfsd_set(new->cap_effective,
new->cap_permitted); new->cap_permitted);
validate_process_creds();
put_cred(override_creds(new)); put_cred(override_creds(new));
put_cred(new); put_cred(new);
validate_process_creds();
return 0; return 0;
oom: oom:
......
...@@ -496,7 +496,9 @@ nfsd(void *vrqstp) ...@@ -496,7 +496,9 @@ nfsd(void *vrqstp)
/* Lock the export hash tables for reading. */ /* Lock the export hash tables for reading. */
exp_readlock(); exp_readlock();
validate_process_creds();
svc_process(rqstp); svc_process(rqstp);
validate_process_creds();
/* Unlock export hash tables */ /* Unlock export hash tables */
exp_readunlock(); exp_readunlock();
......
...@@ -684,6 +684,8 @@ nfsd_open(struct svc_rqst *rqstp, struct svc_fh *fhp, int type, ...@@ -684,6 +684,8 @@ nfsd_open(struct svc_rqst *rqstp, struct svc_fh *fhp, int type,
__be32 err; __be32 err;
int host_err; int host_err;
validate_process_creds();
/* /*
* If we get here, then the client has already done an "open", * If we get here, then the client has already done an "open",
* and (hopefully) checked permission - so allow OWNER_OVERRIDE * and (hopefully) checked permission - so allow OWNER_OVERRIDE
...@@ -740,6 +742,7 @@ nfsd_open(struct svc_rqst *rqstp, struct svc_fh *fhp, int type, ...@@ -740,6 +742,7 @@ nfsd_open(struct svc_rqst *rqstp, struct svc_fh *fhp, int type,
out_nfserr: out_nfserr:
err = nfserrno(host_err); err = nfserrno(host_err);
out: out:
validate_process_creds();
return err; return err;
} }
......
...@@ -959,6 +959,8 @@ struct file *dentry_open(struct dentry *dentry, struct vfsmount *mnt, int flags, ...@@ -959,6 +959,8 @@ struct file *dentry_open(struct dentry *dentry, struct vfsmount *mnt, int flags,
int error; int error;
struct file *f; struct file *f;
validate_creds(cred);
/* /*
* We must always pass in a valid mount pointer. Historically * We must always pass in a valid mount pointer. Historically
* callers got away with not passing it, but we must enforce this at * callers got away with not passing it, but we must enforce this at
......
...@@ -114,6 +114,13 @@ struct thread_group_cred { ...@@ -114,6 +114,13 @@ struct thread_group_cred {
*/ */
struct cred { struct cred {
atomic_t usage; atomic_t usage;
#ifdef CONFIG_DEBUG_CREDENTIALS
atomic_t subscribers; /* number of processes subscribed */
void *put_addr;
unsigned magic;
#define CRED_MAGIC 0x43736564
#define CRED_MAGIC_DEAD 0x44656144
#endif
uid_t uid; /* real UID of the task */ uid_t uid; /* real UID of the task */
gid_t gid; /* real GID of the task */ gid_t gid; /* real GID of the task */
uid_t suid; /* saved UID of the task */ uid_t suid; /* saved UID of the task */
...@@ -143,6 +150,7 @@ struct cred { ...@@ -143,6 +150,7 @@ struct cred {
}; };
extern void __put_cred(struct cred *); extern void __put_cred(struct cred *);
extern void exit_creds(struct task_struct *);
extern int copy_creds(struct task_struct *, unsigned long); extern int copy_creds(struct task_struct *, unsigned long);
extern struct cred *prepare_creds(void); extern struct cred *prepare_creds(void);
extern struct cred *prepare_exec_creds(void); extern struct cred *prepare_exec_creds(void);
...@@ -158,6 +166,60 @@ extern int set_security_override_from_ctx(struct cred *, const char *); ...@@ -158,6 +166,60 @@ extern int set_security_override_from_ctx(struct cred *, const char *);
extern int set_create_files_as(struct cred *, struct inode *); extern int set_create_files_as(struct cred *, struct inode *);
extern void __init cred_init(void); extern void __init cred_init(void);
/*
* check for validity of credentials
*/
#ifdef CONFIG_DEBUG_CREDENTIALS
extern void __invalid_creds(const struct cred *, const char *, unsigned);
extern void __validate_process_creds(struct task_struct *,
const char *, unsigned);
static inline bool creds_are_invalid(const struct cred *cred)
{
if (cred->magic != CRED_MAGIC)
return true;
if (atomic_read(&cred->usage) < atomic_read(&cred->subscribers))
return true;
#ifdef CONFIG_SECURITY_SELINUX
if ((unsigned long) cred->security < PAGE_SIZE)
return true;
if ((*(u32*)cred->security & 0xffffff00) ==
(POISON_FREE << 24 | POISON_FREE << 16 | POISON_FREE << 8))
return true;
#endif
return false;
}
static inline void __validate_creds(const struct cred *cred,
const char *file, unsigned line)
{
if (unlikely(creds_are_invalid(cred)))
__invalid_creds(cred, file, line);
}
#define validate_creds(cred) \
do { \
__validate_creds((cred), __FILE__, __LINE__); \
} while(0)
#define validate_process_creds() \
do { \
__validate_process_creds(current, __FILE__, __LINE__); \
} while(0)
extern void validate_creds_for_do_exit(struct task_struct *);
#else
static inline void validate_creds(const struct cred *cred)
{
}
static inline void validate_creds_for_do_exit(struct task_struct *tsk)
{
}
static inline void validate_process_creds(void)
{
}
#endif
/** /**
* get_new_cred - Get a reference on a new set of credentials * get_new_cred - Get a reference on a new set of credentials
* @cred: The new credentials to reference * @cred: The new credentials to reference
...@@ -187,6 +249,7 @@ static inline struct cred *get_new_cred(struct cred *cred) ...@@ -187,6 +249,7 @@ static inline struct cred *get_new_cred(struct cred *cred)
static inline const struct cred *get_cred(const struct cred *cred) static inline const struct cred *get_cred(const struct cred *cred)
{ {
struct cred *nonconst_cred = (struct cred *) cred; struct cred *nonconst_cred = (struct cred *) cred;
validate_creds(cred);
return get_new_cred(nonconst_cred); return get_new_cred(nonconst_cred);
} }
...@@ -205,7 +268,7 @@ static inline void put_cred(const struct cred *_cred) ...@@ -205,7 +268,7 @@ static inline void put_cred(const struct cred *_cred)
{ {
struct cred *cred = (struct cred *) _cred; struct cred *cred = (struct cred *) _cred;
BUG_ON(atomic_read(&(cred)->usage) <= 0); validate_creds(cred);
if (atomic_dec_and_test(&(cred)->usage)) if (atomic_dec_and_test(&(cred)->usage))
__put_cred(cred); __put_cred(cred);
} }
......
This diff is collapsed.
...@@ -901,6 +901,8 @@ NORET_TYPE void do_exit(long code) ...@@ -901,6 +901,8 @@ NORET_TYPE void do_exit(long code)
tracehook_report_exit(&code); tracehook_report_exit(&code);
validate_creds_for_do_exit(tsk);
/* /*
* We're taking recursive faults here in do_exit. Safest is to just * We're taking recursive faults here in do_exit. Safest is to just
* leave this task alone and wait for reboot. * leave this task alone and wait for reboot.
...@@ -1009,6 +1011,8 @@ NORET_TYPE void do_exit(long code) ...@@ -1009,6 +1011,8 @@ NORET_TYPE void do_exit(long code)
if (tsk->splice_pipe) if (tsk->splice_pipe)
__free_pipe_info(tsk->splice_pipe); __free_pipe_info(tsk->splice_pipe);
validate_creds_for_do_exit(tsk);
preempt_disable(); preempt_disable();
/* causes final put_task_struct in finish_task_switch(). */ /* causes final put_task_struct in finish_task_switch(). */
tsk->state = TASK_DEAD; tsk->state = TASK_DEAD;
......
...@@ -152,8 +152,7 @@ void __put_task_struct(struct task_struct *tsk) ...@@ -152,8 +152,7 @@ void __put_task_struct(struct task_struct *tsk)
WARN_ON(atomic_read(&tsk->usage)); WARN_ON(atomic_read(&tsk->usage));
WARN_ON(tsk == current); WARN_ON(tsk == current);
put_cred(tsk->real_cred); exit_creds(tsk);
put_cred(tsk->cred);
delayacct_tsk_free(tsk); delayacct_tsk_free(tsk);
if (!profile_handoff_task(tsk)) if (!profile_handoff_task(tsk))
...@@ -1307,8 +1306,7 @@ static struct task_struct *copy_process(unsigned long clone_flags, ...@@ -1307,8 +1306,7 @@ static struct task_struct *copy_process(unsigned long clone_flags,
module_put(task_thread_info(p)->exec_domain->module); module_put(task_thread_info(p)->exec_domain->module);
bad_fork_cleanup_count: bad_fork_cleanup_count:
atomic_dec(&p->cred->user->processes); atomic_dec(&p->cred->user->processes);
put_cred(p->real_cred); exit_creds(p);
put_cred(p->cred);
bad_fork_free: bad_fork_free:
free_task(p); free_task(p);
fork_out: fork_out:
......
...@@ -466,6 +466,7 @@ int call_usermodehelper_exec(struct subprocess_info *sub_info, ...@@ -466,6 +466,7 @@ int call_usermodehelper_exec(struct subprocess_info *sub_info,
int retval = 0; int retval = 0;
BUG_ON(atomic_read(&sub_info->cred->usage) != 1); BUG_ON(atomic_read(&sub_info->cred->usage) != 1);
validate_creds(sub_info->cred);
helper_lock(); helper_lock();
if (sub_info->path[0] == '\0') if (sub_info->path[0] == '\0')
......
...@@ -653,6 +653,21 @@ config DEBUG_NOTIFIERS ...@@ -653,6 +653,21 @@ config DEBUG_NOTIFIERS
This is a relatively cheap check but if you care about maximum This is a relatively cheap check but if you care about maximum
performance, say N. performance, say N.
config DEBUG_CREDENTIALS
bool "Debug credential management"
depends on DEBUG_KERNEL
help
Enable this to turn on some debug checking for credential
management. The additional code keeps track of the number of
pointers from task_structs to any given cred struct, and checks to
see that this number never exceeds the usage count of the cred
struct.
Furthermore, if SELinux is enabled, this also checks that the
security pointer in the cred struct is never seen to be invalid.
If unsure, say N.
# #
# Select this config option from the architecture Kconfig, if it # Select this config option from the architecture Kconfig, if it
# it is preferred to always offer frame pointers as a config # it is preferred to always offer frame pointers as a config
......
...@@ -1531,6 +1531,8 @@ static int inode_has_perm(const struct cred *cred, ...@@ -1531,6 +1531,8 @@ static int inode_has_perm(const struct cred *cred,
struct common_audit_data ad; struct common_audit_data ad;
u32 sid; u32 sid;
validate_creds(cred);
if (unlikely(IS_PRIVATE(inode))) if (unlikely(IS_PRIVATE(inode)))
return 0; return 0;
...@@ -3236,7 +3238,9 @@ static int selinux_task_create(unsigned long clone_flags) ...@@ -3236,7 +3238,9 @@ static int selinux_task_create(unsigned long clone_flags)
static void selinux_cred_free(struct cred *cred) static void selinux_cred_free(struct cred *cred)
{ {
struct task_security_struct *tsec = cred->security; struct task_security_struct *tsec = cred->security;
cred->security = NULL;
BUG_ON((unsigned long) cred->security < PAGE_SIZE);
cred->security = (void *) 0x7UL;
kfree(tsec); kfree(tsec);
} }
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment