All of lore.kernel.org
 help / color / mirror / Atom feed
From: Arnd Bergmann <arnd@arndb.de>
To: Scott Bauer <scott.bauer@intel.com>
Cc: keith.busch@intel.com, hch@infradead.org,
	linux-kernel@vger.kernel.org, linux-nvme@lists.infradead.org,
	axboe@fb.com, David.Laight@aculab.com,
	linux-block@vger.kernel.org, jonathan.derrick@intel.com
Subject: Re: [PATCH V3 2/2] Move stack parameters for sed_ioctl to prevent oversized stack with CONFIG_KASAN
Date: Fri, 10 Feb 2017 09:01:23 +0100	[thread overview]
Message-ID: <4455227.qpXP0b5apU@wuerfel> (raw)
In-Reply-To: <1486660801-5105-3-git-send-email-scott.bauer@intel.com>

On Thursday, February 9, 2017 10:20:01 AM CET Scott Bauer wrote:
> When CONFIG_KASAN is enabled, compilation fails:
> 
> block/sed-opal.c: In function 'sed_ioctl':
> block/sed-opal.c:2447:1: error: the frame size of 2256 bytes is larger than 2048 bytes [-Werror=frame-larger-than=]
> 
> Moved all the ioctl structures off the stack and dynamically activate
> using _IOC_SIZE()
> 
> Fixes: 455a7b238cd6 ("block: Add Sed-opal library")
> 
> Reported-by: Arnd Bergmann <arnd@arndb.de>
> Signed-off-by: Scott Bauer <scott.bauer@intel.com>
> ---
>  block/sed-opal.c | 134 +++++++++++++++++++++----------------------------------
>  1 file changed, 50 insertions(+), 84 deletions(-)
> 
> diff --git a/block/sed-opal.c b/block/sed-opal.c
> index bf1406e..4985d95 100644
> --- a/block/sed-opal.c
> +++ b/block/sed-opal.c
> @@ -2346,7 +2346,10 @@ EXPORT_SYMBOL(opal_unlock_from_suspend);
>  
>  int sed_ioctl(struct opal_dev *dev, unsigned int cmd, unsigned long ptr)
>  {
> +	void *ioctl_ptr;
> +	int ret = -ENOTTY;
>  	void __user *arg = (void __user *)ptr;
> +	unsigned int cmd_size = _IOC_SIZE(cmd);
>  
>  	if (!capable(CAP_SYS_ADMIN))
>  		return -EACCES;

We usually have a size check in there to avoid allocating large amounts
of memory. _IOC_SIZEBITS is 14, so you can have up to 16kb here, which
is probably ok, but I'd recommend either adding a comment to say that
it is, or just checking against the largest realistic size.

Having something like v4l with their tables if ioctl commands and
function pointers would also solve it, as you'd be checking for valid
command numbers before doing the copy then.

Otherwise looks good to me.

	Arnd

_______________________________________________
Linux-nvme mailing list
Linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme

WARNING: multiple messages have this Message-ID (diff)
From: arnd@arndb.de (Arnd Bergmann)
Subject: [PATCH V3 2/2] Move stack parameters for sed_ioctl to prevent oversized stack with CONFIG_KASAN
Date: Fri, 10 Feb 2017 09:01:23 +0100	[thread overview]
Message-ID: <4455227.qpXP0b5apU@wuerfel> (raw)
In-Reply-To: <1486660801-5105-3-git-send-email-scott.bauer@intel.com>

On Thursday, February 9, 2017 10:20:01 AM CET Scott Bauer wrote:
> When CONFIG_KASAN is enabled, compilation fails:
> 
> block/sed-opal.c: In function 'sed_ioctl':
> block/sed-opal.c:2447:1: error: the frame size of 2256 bytes is larger than 2048 bytes [-Werror=frame-larger-than=]
> 
> Moved all the ioctl structures off the stack and dynamically activate
> using _IOC_SIZE()
> 
> Fixes: 455a7b238cd6 ("block: Add Sed-opal library")
> 
> Reported-by: Arnd Bergmann <arnd at arndb.de>
> Signed-off-by: Scott Bauer <scott.bauer at intel.com>
> ---
>  block/sed-opal.c | 134 +++++++++++++++++++++----------------------------------
>  1 file changed, 50 insertions(+), 84 deletions(-)
> 
> diff --git a/block/sed-opal.c b/block/sed-opal.c
> index bf1406e..4985d95 100644
> --- a/block/sed-opal.c
> +++ b/block/sed-opal.c
> @@ -2346,7 +2346,10 @@ EXPORT_SYMBOL(opal_unlock_from_suspend);
>  
>  int sed_ioctl(struct opal_dev *dev, unsigned int cmd, unsigned long ptr)
>  {
> +	void *ioctl_ptr;
> +	int ret = -ENOTTY;
>  	void __user *arg = (void __user *)ptr;
> +	unsigned int cmd_size = _IOC_SIZE(cmd);
>  
>  	if (!capable(CAP_SYS_ADMIN))
>  		return -EACCES;

We usually have a size check in there to avoid allocating large amounts
of memory. _IOC_SIZEBITS is 14, so you can have up to 16kb here, which
is probably ok, but I'd recommend either adding a comment to say that
it is, or just checking against the largest realistic size.

Having something like v4l with their tables if ioctl commands and
function pointers would also solve it, as you'd be checking for valid
command numbers before doing the copy then.

Otherwise looks good to me.

	Arnd

  parent reply	other threads:[~2017-02-10  8:01 UTC|newest]

Thread overview: 26+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-02-09 17:19 Sed-opal fixups Scott Bauer
2017-02-09 17:19 ` Scott Bauer
2017-02-09 17:20 ` [PATCH V3 1/2] uapi: sed-opal fix IOW for activate lsp to use correct struct Scott Bauer
2017-02-09 17:20   ` Scott Bauer
2017-02-09 17:20 ` [PATCH V3 2/2] Move stack parameters for sed_ioctl to prevent oversized stack with CONFIG_KASAN Scott Bauer
2017-02-09 17:20   ` Scott Bauer
2017-02-09 19:51   ` Rafael Antognolli
2017-02-09 19:51     ` Rafael Antognolli
2017-02-10  7:45   ` Johannes Thumshirn
2017-02-10  7:45     ` Johannes Thumshirn
2017-02-10 10:28     ` David Laight
2017-02-10 10:28       ` David Laight
2017-02-10 11:00       ` Arnd Bergmann
2017-02-10 11:00         ` Arnd Bergmann
2017-02-10  8:01   ` Arnd Bergmann [this message]
2017-02-10  8:01     ` Arnd Bergmann
2017-02-10 15:57     ` Scott Bauer
2017-02-10 15:57       ` Scott Bauer
2017-02-09 17:43 ` Sed-opal fixups David Laight
2017-02-09 17:43   ` David Laight
2017-02-09 17:45   ` Scott Bauer
2017-02-09 17:45     ` Scott Bauer
2017-02-09 18:24     ` Jens Axboe
2017-02-09 18:24       ` Jens Axboe
2017-02-09 20:01       ` Scott Bauer
2017-02-09 20:01         ` Scott Bauer

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=4455227.qpXP0b5apU@wuerfel \
    --to=arnd@arndb.de \
    --cc=David.Laight@aculab.com \
    --cc=axboe@fb.com \
    --cc=hch@infradead.org \
    --cc=jonathan.derrick@intel.com \
    --cc=keith.busch@intel.com \
    --cc=linux-block@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-nvme@lists.infradead.org \
    --cc=scott.bauer@intel.com \
    /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.