Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 31e33a5b authored by Jann Horn's avatar Jann Horn Committed by Saeed Mahameed
Browse files

net/mlx5: fix uaccess beyond "count" in debugfs read/write handlers 



In general, accessing userspace memory beyond the length of the supplied
buffer in VFS read/write handlers can lead to both kernel memory corruption
(via kernel_read()/kernel_write(), which can e.g. be triggered via
sys_splice()) and privilege escalation inside userspace.

In this case, the affected files are in debugfs (and should therefore only
be accessible to root) and check that *pos is zero (which prevents the
sys_splice() trick). Therefore, this is not a security fix, but rather a
small cleanup.

For the read handlers, fix it by using simple_read_from_buffer() instead of
custom logic.
For the write handler, add a check.

changed in v2:
 - also fix dbg_write()

Fixes: e126ba97 ("mlx5: Add driver for Mellanox Connect-IB adapters")
Signed-off-by: default avatarJann Horn <jannh@google.com>
Reviewed-by: default avatarLeon Romanovsky <leonro@mellanox.com>
Signed-off-by: default avatarSaeed Mahameed <saeedm@mellanox.com>
parent b183ee27

drivers/net/ethernet/mellanox/mlx5/core/cmd.c

+7 −21
Original line number Diff line number Diff line
@@ -1032,7 +1032,10 @@ static ssize_t dbg_write(struct file *filp, const char __user *buf, 
	if (!dbg->in_msg || !dbg->out_msg)

		return -ENOMEM;



	if (copy_from_user(lbuf, buf, sizeof(lbuf)))

	if (count < sizeof(lbuf) - 1)

		return -EINVAL;



	if (copy_from_user(lbuf, buf, sizeof(lbuf) - 1))

		return -EFAULT;



	lbuf[sizeof(lbuf) - 1] = 0;
@@ -1236,21 +1239,12 @@ static ssize_t data_read(struct file *filp, char __user *buf, size_t count, 
{

	struct mlx5_core_dev *dev = filp->private_data;

	struct mlx5_cmd_debug *dbg = &dev->cmd.dbg;

	int copy;



	if (*pos)

		return 0;



	if (!dbg->out_msg)

		return -ENOMEM;



	copy = min_t(int, count, dbg->outlen);

	if (copy_to_user(buf, dbg->out_msg, copy))

		return -EFAULT;



	*pos += copy;



	return copy;

	return simple_read_from_buffer(buf, count, pos, dbg->out_msg,

				       dbg->outlen);

}



static const struct file_operations dfops = {
@@ -1268,19 +1262,11 @@ static ssize_t outlen_read(struct file *filp, char __user *buf, size_t count, 
	char outlen[8];

	int err;



	if (*pos)

		return 0;



	err = snprintf(outlen, sizeof(outlen), "%d", dbg->outlen);

	if (err < 0)

		return err;



	if (copy_to_user(buf, &outlen, err))

		return -EFAULT;



	*pos += err;



	return err;

	return simple_read_from_buffer(buf, count, pos, outlen, err);

}



static ssize_t outlen_write(struct file *filp, const char __user *buf,

drivers/net/ethernet/mellanox/mlx5/core/debugfs.c

+2 −20
Original line number Diff line number Diff line
@@ -150,22 +150,13 @@ static ssize_t average_read(struct file *filp, char __user *buf, size_t count, 
	int ret;

	char tbuf[22];



	if (*pos)

		return 0;



	stats = filp->private_data;

	spin_lock_irq(&stats->lock);

	if (stats->n)

		field = div64_u64(stats->sum, stats->n);

	spin_unlock_irq(&stats->lock);

	ret = snprintf(tbuf, sizeof(tbuf), "%llu\n", field);

	if (ret > 0) {

		if (copy_to_user(buf, tbuf, ret))

			return -EFAULT;

	}



	*pos += ret;

	return ret;

	return simple_read_from_buffer(buf, count, pos, tbuf, ret);

}



static ssize_t average_write(struct file *filp, const char __user *buf,
@@ -442,9 +433,6 @@ static ssize_t dbg_read(struct file *filp, char __user *buf, size_t count, 
	u64 field;

	int ret;



	if (*pos)

		return 0;



	desc = filp->private_data;

	d = (void *)(desc - desc->i) - sizeof(*d);

	switch (d->type) {
@@ -470,13 +458,7 @@ static ssize_t dbg_read(struct file *filp, char __user *buf, size_t count, 
	else

		ret = snprintf(tbuf, sizeof(tbuf), "0x%llx\n", field);



	if (ret > 0) {

		if (copy_to_user(buf, tbuf, ret))

			return -EFAULT;

	}



	*pos += ret;

	return ret;

	return simple_read_from_buffer(buf, count, pos, tbuf, ret);

}



static const struct file_operations fops = {