On Fri, Apr 12, 2019 at 10:08:48PM +0800, Liu, Changcheng wrote:
> 1. pr_err_once only print once after it's called. However,
> dmesg output is flushed after long time. It's hard to get
> this info without occurring anymore. Use pr_err to print
> the access permission not allow info.
>
> 2. unify ib_safe_file_access check implementation.
>
> Signed-off-by: Changcheng Liu <changcheng.liu@xxxxxxxxx>
>
> diff --git a/drivers/infiniband/core/ucm.c b/drivers/infiniband/core/ucm.c
> index 7541fbaf58a3..2df984f10a2c 100644
> +++ b/drivers/infiniband/core/ucm.c
> @@ -1108,11 +1108,9 @@ static ssize_t ib_ucm_write(struct file *filp, const char __user *buf,
> struct ib_ucm_cmd_hdr hdr;
> ssize_t result;
>
> - if (!ib_safe_file_access(filp)) {
> - pr_err_once("ucm_write: process %d (%s) changed security contexts after opening file descriptor, this is not allowed.\n",
> - task_tgid_vnr(current), current->comm);
> - return -EACCES;
> - }
> + result = ib_safe_file_access_check(filp);
> + if (result == -EACCES)
> + return result;
>
> if (len < sizeof(hdr))
> return -EINVAL;
> diff --git a/drivers/infiniband/core/ucma.c b/drivers/infiniband/core/ucma.c
> index 01d68ed46c1b..7b298ff3e4a5 100644
> +++ b/drivers/infiniband/core/ucma.c
> @@ -1664,11 +1664,9 @@ static ssize_t ucma_write(struct file *filp, const char __user *buf,
> struct rdma_ucm_cmd_hdr hdr;
> ssize_t ret;
>
> - if (!ib_safe_file_access(filp)) {
> - pr_err_once("ucma_write: process %d (%s) changed security contexts after opening file descriptor, this is not allowed.\n",
> - task_tgid_vnr(current), current->comm);
> - return -EACCES;
> - }
> + ret = ib_safe_file_access_check(filp);
> + if (ret == -EACCES)
> + return ret;
>
> if (len < sizeof(hdr))
> return -EINVAL;
> diff --git a/drivers/infiniband/core/uverbs_main.c b/drivers/infiniband/core/uverbs_main.c
> index 5f366838b7ff..71dc597c9f4e 100644
> +++ b/drivers/infiniband/core/uverbs_main.c
> @@ -662,11 +662,9 @@ static ssize_t ib_uverbs_write(struct file *filp, const char __user *buf,
> int srcu_key;
> ssize_t ret;
>
> - if (!ib_safe_file_access(filp)) {
> - pr_err_once("uverbs_write: process %d (%s) changed security contexts after opening file descriptor, this is not allowed.\n",
> - task_tgid_vnr(current), current->comm);
> - return -EACCES;
> - }
> + ret = ib_safe_file_access_check(filp);
> + if (ret == -EACCES)
> + return ret;
>
> if (count < sizeof(hdr))
> return -EINVAL;
> diff --git a/drivers/infiniband/hw/qib/qib_file_ops.c b/drivers/infiniband/hw/qib/qib_file_ops.c
> index 78fa634de98a..5ed15dec9cdb 100644
> +++ b/drivers/infiniband/hw/qib/qib_file_ops.c
> @@ -2043,11 +2043,9 @@ static ssize_t qib_write(struct file *fp, const char __user *data,
> ssize_t ret = 0;
> void *dest;
>
> - if (!ib_safe_file_access(fp)) {
> - pr_err_once("qib_write: process %d (%s) changed security contexts after opening file descriptor, this is not allowed.\n",
> - task_tgid_vnr(current), current->comm);
> - return -EACCES;
> - }
> + ret = ib_safe_file_access_check(fp);
> + if (ret == -EACCES)
> + return ret;
>
> if (count < sizeof(cmd.type)) {
> ret = -EINVAL;
> diff --git a/include/rdma/ib.h b/include/rdma/ib.h
> index 4f385ec54f80..736c67530107 100644
> +++ b/include/rdma/ib.h
> @@ -95,12 +95,19 @@ struct sockaddr_ib {
> * traditional suid executable error message writes, but also various kernel
> * interfaces that can write to file descriptors.
> *
> - * This function provides protection for the legacy API by restricting the
> + * This macro provides protection for the legacy API by restricting the
> * calling context.
> */
> -static inline bool ib_safe_file_access(struct file *filp)
> -{
> - return filp->f_cred == current_cred() && !uaccess_kernel();
> -}
> +#define ib_safe_file_access_check(filp) \
> +({ \
> + ssize_t rst = 0; \
> + if (!filp->f_cred == current_cred() || uaccess_kernel()) { \
> + pr_err("%s: process %d (%s) changed security contexts " \
> + "opening file descriptor, it's not allowed.\n", \
> + __func__, task_tgid_vnr(current), current->comm); \
> + rst = -EACCES; \
> + } \
> + rst; \
> +})
We can't let user space spam the log.
Jason
