Hi,
On 7/20/22 20:23, Dan Carpenter wrote:
> The call to:
>
> size = simple_write_to_buffer(cmdbuf, sizeof(cmdbuf), ppos, buf, size);
>
> will succeed if at least one byte is written to the "cmdbuf" buffer.
> The "*ppos" value controls which byte is written. Another problem is
> that this code does not check for errors so it's possible for the entire
> buffer to be unintialized.
>
> Inintialize the struct to zero to prevent reading uninitialized stack
> data.
>
> Debugfs is normally only writable by root so the impact of this bug is
> very minimal.
>
> Fixes: 6cca83d498bd ("Platform: OLPC: move debugfs support from x86 EC driver")
> Signed-off-by: Dan Carpenter <dan.carpenter@xxxxxxxxxx>
Thank you for your patch, I've applied this patch to my review-hans
branch:
https://git.kernel.org/pub/scm/linux/kernel/git/pdx86/platform-drivers-x86.git/log/?h=review-hans
Note it will show up in my review-hans branch once I've pushed my
local branch there, which might take a while.
Once I've run some tests on this branch the patches there will be
added to the platform-drivers-x86/for-next branch and eventually
will be included in the pdx86 pull-request to Linus for the next
merge-window.
Regards,
Hans
> ---
> The ec_dbgfs_cmd_write() function is not great. We could copy the data
> outside the lock for example. But that's outside the scope of this
> patch.
>
> drivers/platform/olpc/olpc-ec.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/platform/olpc/olpc-ec.c b/drivers/platform/olpc/olpc-ec.c
> index 4ff5c3a12991..921520475ff6 100644
> --- a/drivers/platform/olpc/olpc-ec.c
> +++ b/drivers/platform/olpc/olpc-ec.c
> @@ -264,7 +264,7 @@ static ssize_t ec_dbgfs_cmd_write(struct file *file, const char __user *buf,
> int i, m;
> unsigned char ec_cmd[EC_MAX_CMD_ARGS];
> unsigned int ec_cmd_int[EC_MAX_CMD_ARGS];
> - char cmdbuf[64];
> + char cmdbuf[64] = "";
> int ec_cmd_bytes;
>
> mutex_lock(&ec_dbgfs_lock);
