On Fri, 29 Jan 2016 12:13:47 +0100, Vinod Koul wrote:
On Fri, Jan 29, 2016 at 12:21:06PM +0530, Vinod Koul wrote:
On Thu, Jan 28, 2016 at 05:19:21PM +0100, Takashi Iwai wrote:
The line number doesn't match with the latest code in git, so double-check that the problem happens with the latest alsa-lib and alsa-utils, too.
I am on debian packages 1.0.27
I'm thinking whether this is rather an issue in the kernel driver side. In skl_tplg_tlv_control_get(),
I think you are right, the buffer would overflow which would cause heap to go bad and free goes crashing
if (bc->params) { if (copy_to_user(data, &bc->param_id, sizeof(u32))) return -EFAULT; if (copy_to_user(data + 1, &size, sizeof(u32))) return -EFAULT; if (copy_to_user(data + 2, bc->params, size)) return -EFAULT; }
But here, size is the size of the whole container, not the size in the container. In the code above, you're copying size+8 bytes total and this breaks the boundary already.
Right, also I think we need to check for size vs size of parameters. We don't want to copy kernel memory to usermode if usermode gave a larger buffer
Let me test this, thanks for pointing
And you were right :)
Good to hear :)
with this change it works and dumps 4K bytes on my screen
@@ -913,6 +913,13 @@ static int skl_tplg_tlv_control_get(struct snd_kcontrol *kcontrol, skl_get_module_params(skl->skl_sst, (u32 *)bc->params, bc->max, bc->param_id, mconfig);
/* decrement size for TLV header */size -= 2 * sizeof(u32);
The lower bound check is also missing.
if (size < 2 * sizeof(u32)) return -EINVAL;
Takashi
/* check size as we don't want to send kernel data */if (size > bc->max)size = bc->max;if (bc->params) { if (copy_to_user(data, &bc->param_id, sizeof(u32))) return -EFAULT;Thanks
~Vinod