Commit 6b4f3d01 authored by Stephen Smalley's avatar Stephen Smalley Committed by James Morris
Browse files

usb, signal, security: only pass the cred, not the secid, to... 


usb, signal, security: only pass the cred, not the secid, to kill_pid_info_as_cred and security_task_kill

commit d178bc3a ("user namespace: usb:
 make usb urbs user namespace aware (v2)") changed kill_pid_info_as_uid
to kill_pid_info_as_cred, saving and passing a cred structure instead of
uids.  Since the secid can be obtained from the cred, drop the secid fields
from the usb_dev_state and async structures, and drop the secid argument to
kill_pid_info_as_cred.  Replace the secid argument to security_task_kill
with the cred.  Update SELinux, Smack, and AppArmor to use the cred, which
avoids the need for Smack and AppArmor to use a secid at all in this hook.
Further changes to Smack might still be required to take full advantage of
this change, since it should now be possible to perform capability
checking based on the supplied cred.  The changes to Smack and AppArmor
have only been compile-tested.

Signed-off-by: default avatarStephen Smalley <sds@tycho.nsa.gov>
Acked-by: default avatarPaul Moore <paul@paul-moore.com>
Acked-by: default avatarCasey Schaufler <casey@schaufler-ca.com>
Acked-by: default avatarGreg Kroah-Hartman <gregkh@linuxfoundation.org>
Acked-by: default avatarJohn Johansen <john.johansen@canonical.com>
Signed-off-by: default avatarJames Morris <james.morris@microsoft.com>
parent a02633e9

drivers/usb/core/devio.c

+2 −8
Original line number Diff line number Diff line
@@ -65,7 +65,6 @@ struct usb_dev_state { 
	const struct cred *cred;

	void __user *disccontext;

	unsigned long ifclaimed;

	u32 secid;

	u32 disabled_bulk_eps;

	bool privileges_dropped;

	unsigned long interface_allowed_mask;
@@ -95,7 +94,6 @@ struct async { 
	struct usb_memory *usbm;

	unsigned int mem_usage;

	int status;

	u32 secid;

	u8 bulk_addr;

	u8 bulk_status;

};
@@ -586,7 +584,6 @@ static void async_completed(struct urb *urb) 
	struct usb_dev_state *ps = as->ps;

	struct siginfo sinfo;

	struct pid *pid = NULL;

	u32 secid = 0;

	const struct cred *cred = NULL;

	int signr;
@@ -602,7 +599,6 @@ static void async_completed(struct urb *urb) 
		sinfo.si_addr = as->userurb;

		pid = get_pid(as->pid);

		cred = get_cred(as->cred);

		secid = as->secid;

	}

	snoop(&urb->dev->dev, "urb complete\n");

	snoop_urb(urb->dev, as->userurb, urb->pipe, urb->actual_length,
@@ -618,7 +614,7 @@ static void async_completed(struct urb *urb) 
	spin_unlock(&ps->lock);



	if (signr) {

		kill_pid_info_as_cred(sinfo.si_signo, &sinfo, pid, cred, secid);

		kill_pid_info_as_cred(sinfo.si_signo, &sinfo, pid, cred);

		put_pid(pid);

		put_cred(cred);

	}
@@ -1013,7 +1009,6 @@ static int usbdev_open(struct inode *inode, struct file *file) 
	init_waitqueue_head(&ps->wait);

	ps->disc_pid = get_pid(task_pid(current));

	ps->cred = get_current_cred();

	security_task_getsecid(current, &ps->secid);

	smp_wmb();

	list_add_tail(&ps->list, &dev->filelist);

	file->private_data = ps;
@@ -1727,7 +1722,6 @@ static int proc_do_submiturb(struct usb_dev_state *ps, struct usbdevfs_urb *uurb 
	as->ifnum = ifnum;

	as->pid = get_pid(task_pid(current));

	as->cred = get_current_cred();

	security_task_getsecid(current, &as->secid);

	snoop_urb(ps->dev, as->userurb, as->urb->pipe,

			as->urb->transfer_buffer_length, 0, SUBMIT,

			NULL, 0);
@@ -2617,7 +2611,7 @@ static void usbdev_remove(struct usb_device *udev) 
			sinfo.si_code = SI_ASYNCIO;

			sinfo.si_addr = ps->disccontext;

			kill_pid_info_as_cred(ps->discsignr, &sinfo,

					ps->disc_pid, ps->cred, ps->secid);

					ps->disc_pid, ps->cred);

		}

	}

}

include/linux/lsm_hooks.h

+3 −2
Original line number Diff line number Diff line
@@ -672,7 +672,8 @@ 
 *	@p contains the task_struct for process.

 *	@info contains the signal information.

 *	@sig contains the signal value.

 *	@secid contains the sid of the process where the signal originated

 *	@cred contains the cred of the process where the signal originated, or

 *	NULL if the current task is the originator.

 *	Return 0 if permission is granted.

 * @task_prctl:

 *	Check permission before performing a process control operation on the
@@ -1564,7 +1565,7 @@ union security_list_options { 
	int (*task_getscheduler)(struct task_struct *p);

	int (*task_movememory)(struct task_struct *p);

	int (*task_kill)(struct task_struct *p, struct siginfo *info,

				int sig, u32 secid);

				int sig, const struct cred *cred);

	int (*task_prctl)(int option, unsigned long arg2, unsigned long arg3,

				unsigned long arg4, unsigned long arg5);

	void (*task_to_inode)(struct task_struct *p, struct inode *inode);

include/linux/sched/signal.h

+1 −1
Original line number Diff line number Diff line
@@ -319,7 +319,7 @@ extern int force_sig_info(int, struct siginfo *, struct task_struct *); 
extern int __kill_pgrp_info(int sig, struct siginfo *info, struct pid *pgrp);

extern int kill_pid_info(int sig, struct siginfo *info, struct pid *pid);

extern int kill_pid_info_as_cred(int, struct siginfo *, struct pid *,

				const struct cred *, u32);

				const struct cred *);

extern int kill_pgrp(struct pid *pid, int sig, int priv);

extern int kill_pid(struct pid *pid, int sig, int priv);

extern __must_check bool do_notify_parent(struct task_struct *, int);

include/linux/security.h

+2 −2
Original line number Diff line number Diff line
@@ -347,7 +347,7 @@ int security_task_setscheduler(struct task_struct *p); 
int security_task_getscheduler(struct task_struct *p);

int security_task_movememory(struct task_struct *p);

int security_task_kill(struct task_struct *p, struct siginfo *info,

			int sig, u32 secid);

			int sig, const struct cred *cred);

int security_task_prctl(int option, unsigned long arg2, unsigned long arg3,

			unsigned long arg4, unsigned long arg5);

void security_task_to_inode(struct task_struct *p, struct inode *inode);
@@ -1010,7 +1010,7 @@ static inline int security_task_movememory(struct task_struct *p) 


static inline int security_task_kill(struct task_struct *p,

				     struct siginfo *info, int sig,

				     u32 secid)

				     const struct cred *cred)

{

	return 0;

}

kernel/signal.c

+3 −3
Original line number Diff line number Diff line
@@ -770,7 +770,7 @@ static int check_kill_permission(int sig, struct siginfo *info, 
		}

	}



	return security_task_kill(t, info, sig, 0);

	return security_task_kill(t, info, sig, NULL);

}



/**
@@ -1361,7 +1361,7 @@ static int kill_as_cred_perm(const struct cred *cred, 


/* like kill_pid_info(), but doesn't use uid/euid of "current" */

int kill_pid_info_as_cred(int sig, struct siginfo *info, struct pid *pid,

			 const struct cred *cred, u32 secid)

			 const struct cred *cred)

{

	int ret = -EINVAL;

	struct task_struct *p;
@@ -1380,7 +1380,7 @@ int kill_pid_info_as_cred(int sig, struct siginfo *info, struct pid *pid, 
		ret = -EPERM;

		goto out_unlock;

	}

	ret = security_task_kill(p, info, sig, secid);

	ret = security_task_kill(p, info, sig, cred);

	if (ret)

		goto out_unlock;

CRA Git | Maintained and supported by SUSTech CRA and CCSE