Commit ed9f54cc authored by Caleb Sander Mateos's avatar Caleb Sander Mateos Committed by Jens Axboe
Browse files

ublk: use READ_ONCE() to read struct ublksrv_ctrl_cmd 



struct ublksrv_ctrl_cmd is part of the io_uring_sqe, which may lie in
userspace-mapped memory. It's racy to access its fields with normal
loads, as userspace may write to them concurrently. Use READ_ONCE() to
copy the ublksrv_ctrl_cmd from the io_uring_sqe to the stack. Use the
local copy in place of the one in the io_uring_sqe.

Fixes: 87213b0d ("ublk: allow non-blocking ctrl cmds in IO_URING_F_NONBLOCK issue")
Signed-off-by: default avatarCaleb Sander Mateos <csander@purestorage.com>
Reviewed-by: default avatarMing Lei <ming.lei@redhat.com>
Signed-off-by: default avatarJens Axboe <axboe@kernel.dk>
parent da7e4b75

drivers/block/ublk_drv.c

+31 −25
Original line number Diff line number Diff line
@@ -4731,12 +4731,11 @@ static int ublk_ctrl_del_dev(struct ublk_device **p_ub, bool wait) 
	return 0;

}



static inline void ublk_ctrl_cmd_dump(struct io_uring_cmd *cmd)

static inline void ublk_ctrl_cmd_dump(u32 cmd_op,

				      const struct ublksrv_ctrl_cmd *header)

{

	const struct ublksrv_ctrl_cmd *header = io_uring_sqe_cmd(cmd->sqe);



	pr_devel("%s: cmd_op %x, dev id %d qid %d data %llx buf %llx len %u\n",

			__func__, cmd->cmd_op, header->dev_id, header->queue_id,

			__func__, cmd_op, header->dev_id, header->queue_id,

			header->data[0], header->addr, header->len);

}
@@ -5119,9 +5118,8 @@ static int ublk_char_dev_permission(struct ublk_device *ub, 
}



static int ublk_ctrl_uring_cmd_permission(struct ublk_device *ub,

		struct io_uring_cmd *cmd)

		u32 cmd_op, struct ublksrv_ctrl_cmd *header)

{

	struct ublksrv_ctrl_cmd *header = (struct ublksrv_ctrl_cmd *)io_uring_sqe_cmd(cmd->sqe);

	bool unprivileged = ub->dev_info.flags & UBLK_F_UNPRIVILEGED_DEV;

	void __user *argp = (void __user *)(unsigned long)header->addr;

	char *dev_path = NULL;
@@ -5137,7 +5135,7 @@ static int ublk_ctrl_uring_cmd_permission(struct ublk_device *ub, 
		 * know if the specified device is created as unprivileged

		 * mode.

		 */

		if (_IOC_NR(cmd->cmd_op) != UBLK_CMD_GET_DEV_INFO2)

		if (_IOC_NR(cmd_op) != UBLK_CMD_GET_DEV_INFO2)

			return 0;

	}
@@ -5158,7 +5156,7 @@ static int ublk_ctrl_uring_cmd_permission(struct ublk_device *ub, 
		return PTR_ERR(dev_path);



	ret = -EINVAL;

	switch (_IOC_NR(cmd->cmd_op)) {

	switch (_IOC_NR(cmd_op)) {

	case UBLK_CMD_GET_DEV_INFO:

	case UBLK_CMD_GET_DEV_INFO2:

	case UBLK_CMD_GET_QUEUE_AFFINITY:
@@ -5188,7 +5186,7 @@ static int ublk_ctrl_uring_cmd_permission(struct ublk_device *ub, 
		header->addr += header->dev_path_len;

	}

	pr_devel("%s: dev id %d cmd_op %x uid %d gid %d path %s ret %d\n",

			__func__, ub->ub_number, cmd->cmd_op,

			__func__, ub->ub_number, cmd_op,

			ub->dev_info.owner_uid, ub->dev_info.owner_gid,

			dev_path, ret);

exit:
@@ -5212,7 +5210,9 @@ static bool ublk_ctrl_uring_cmd_may_sleep(u32 cmd_op) 
static int ublk_ctrl_uring_cmd(struct io_uring_cmd *cmd,

		unsigned int issue_flags)

{

	const struct ublksrv_ctrl_cmd *header = io_uring_sqe_cmd(cmd->sqe);

	/* May point to userspace-mapped memory */

	const struct ublksrv_ctrl_cmd *ub_src = io_uring_sqe_cmd(cmd->sqe);

	struct ublksrv_ctrl_cmd header;

	struct ublk_device *ub = NULL;

	u32 cmd_op = cmd->cmd_op;

	int ret = -EINVAL;
@@ -5224,31 +5224,37 @@ static int ublk_ctrl_uring_cmd(struct io_uring_cmd *cmd, 
	if (!(issue_flags & IO_URING_F_SQE128))

		return -EINVAL;



	ublk_ctrl_cmd_dump(cmd);

	header.dev_id = READ_ONCE(ub_src->dev_id);

	header.queue_id = READ_ONCE(ub_src->queue_id);

	header.len = READ_ONCE(ub_src->len);

	header.addr = READ_ONCE(ub_src->addr);

	header.data[0] = READ_ONCE(ub_src->data[0]);

	header.dev_path_len = READ_ONCE(ub_src->dev_path_len);

	ublk_ctrl_cmd_dump(cmd_op, &header);



	ret = ublk_check_cmd_op(cmd_op);

	if (ret)

		goto out;



	if (cmd_op == UBLK_U_CMD_GET_FEATURES) {

		ret = ublk_ctrl_get_features(header);

		ret = ublk_ctrl_get_features(&header);

		goto out;

	}



	if (_IOC_NR(cmd_op) != UBLK_CMD_ADD_DEV) {

		ret = -ENODEV;

		ub = ublk_get_device_from_id(header->dev_id);

		ub = ublk_get_device_from_id(header.dev_id);

		if (!ub)

			goto out;



		ret = ublk_ctrl_uring_cmd_permission(ub, cmd);

		ret = ublk_ctrl_uring_cmd_permission(ub, cmd_op, &header);

		if (ret)

			goto put_dev;

	}



	switch (_IOC_NR(cmd_op)) {

	case UBLK_CMD_START_DEV:

		ret = ublk_ctrl_start_dev(ub, header);

		ret = ublk_ctrl_start_dev(ub, &header);

		break;

	case UBLK_CMD_STOP_DEV:

		ublk_ctrl_stop_dev(ub);
@@ -5256,10 +5262,10 @@ static int ublk_ctrl_uring_cmd(struct io_uring_cmd *cmd, 
		break;

	case UBLK_CMD_GET_DEV_INFO:

	case UBLK_CMD_GET_DEV_INFO2:

		ret = ublk_ctrl_get_dev_info(ub, header);

		ret = ublk_ctrl_get_dev_info(ub, &header);

		break;

	case UBLK_CMD_ADD_DEV:

		ret = ublk_ctrl_add_dev(header);

		ret = ublk_ctrl_add_dev(&header);

		break;

	case UBLK_CMD_DEL_DEV:

		ret = ublk_ctrl_del_dev(&ub, true);
@@ -5268,26 +5274,26 @@ static int ublk_ctrl_uring_cmd(struct io_uring_cmd *cmd, 
		ret = ublk_ctrl_del_dev(&ub, false);

		break;

	case UBLK_CMD_GET_QUEUE_AFFINITY:

		ret = ublk_ctrl_get_queue_affinity(ub, header);

		ret = ublk_ctrl_get_queue_affinity(ub, &header);

		break;

	case UBLK_CMD_GET_PARAMS:

		ret = ublk_ctrl_get_params(ub, header);

		ret = ublk_ctrl_get_params(ub, &header);

		break;

	case UBLK_CMD_SET_PARAMS:

		ret = ublk_ctrl_set_params(ub, header);

		ret = ublk_ctrl_set_params(ub, &header);

		break;

	case UBLK_CMD_START_USER_RECOVERY:

		ret = ublk_ctrl_start_recovery(ub, header);

		ret = ublk_ctrl_start_recovery(ub, &header);

		break;

	case UBLK_CMD_END_USER_RECOVERY:

		ret = ublk_ctrl_end_recovery(ub, header);

		ret = ublk_ctrl_end_recovery(ub, &header);

		break;

	case UBLK_CMD_UPDATE_SIZE:

		ublk_ctrl_set_size(ub, header);

		ublk_ctrl_set_size(ub, &header);

		ret = 0;

		break;

	case UBLK_CMD_QUIESCE_DEV:

		ret = ublk_ctrl_quiesce_dev(ub, header);

		ret = ublk_ctrl_quiesce_dev(ub, &header);

		break;

	case UBLK_CMD_TRY_STOP_DEV:

		ret = ublk_ctrl_try_stop_dev(ub);
@@ -5302,7 +5308,7 @@ static int ublk_ctrl_uring_cmd(struct io_uring_cmd *cmd, 
		ublk_put_device(ub);

 out:

	pr_devel("%s: cmd done ret %d cmd_op %x, dev id %d qid %d\n",

			__func__, ret, cmd->cmd_op, header->dev_id, header->queue_id);

			__func__, ret, cmd_op, header.dev_id, header.queue_id);

	return ret;

}