All of lore.kernel.org
 help / color / mirror / Atom feed
From: Dan Carpenter <dan.carpenter@oracle.com>
To: Hans de Goede <hdegoede@redhat.com>,
	Andres Salomon <dilinger@queued.net>
Cc: Mark Gross <markgross@kernel.org>,
	platform-driver-x86@vger.kernel.org,
	kernel-janitors@vger.kernel.org
Subject: Re: [PATCH] Platform: OLPC: uninitialized data in debugfs write
Date: Wed, 20 Jul 2022 21:33:29 +0300	[thread overview]
Message-ID: <20220720183329.GE2316@kadam> (raw)
In-Reply-To: <YthIKn+TfZSZMEcM@kili>

On Wed, Jul 20, 2022 at 09:23:38PM +0300, 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@oracle.com>
> ---
> 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.

More relevant another improvement would be to replace the
simple_write_to_buffer() with a check for "if (*ppos) return 0;" and
a copy_from_user().  The simple_write_to_buffer() function is not
appropriate here.

However I can't test this code, and this is not really core code so I
just did the minimum to fix the bug.

regards,
dan carpenter


  reply	other threads:[~2022-07-20 18:34 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-07-20 18:23 [PATCH] Platform: OLPC: uninitialized data in debugfs write Dan Carpenter
2022-07-20 18:33 ` Dan Carpenter [this message]
2022-07-28 18:44 ` Hans de Goede

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20220720183329.GE2316@kadam \
    --to=dan.carpenter@oracle.com \
    --cc=dilinger@queued.net \
    --cc=hdegoede@redhat.com \
    --cc=kernel-janitors@vger.kernel.org \
    --cc=markgross@kernel.org \
    --cc=platform-driver-x86@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.