Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1577818 > unrolled thread

[PATCH V3 2/2] Move stack parameters for sed_ioctl to prevent oversized stack with CONFIG_KASAN

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  [PATCH V3 2/2] Move stack parameters for sed_ioctl to prevent oversized stack with CONFIG_KASAN Scott Bauer <scott.bauer@intel.com> - 2017-02-09 18:40 +0100
    Re: [PATCH V3 2/2] Move stack parameters for sed_ioctl to prevent  oversized stack with CONFIG_KASAN Rafael Antognolli <rafael.antognolli@intel.com> - 2017-02-09 21:00 +0100
    Re: [PATCH V3 2/2] Move stack parameters for sed_ioctl to prevent  oversized stack with CONFIG_KASAN Johannes Thumshirn <jthumshirn@suse.de> - 2017-02-10 08:50 +0100
      RE: [PATCH V3 2/2] Move stack parameters for sed_ioctl to prevent  oversized stack with CONFIG_KASAN David Laight <David.Laight@ACULAB.COM> - 2017-02-10 11:50 +0100
        Re: [PATCH V3 2/2] Move stack parameters for sed_ioctl to prevent  oversized stack with CONFIG_KASAN Arnd Bergmann <arnd@arndb.de> - 2017-02-10 12:10 +0100
    Re: [PATCH V3 2/2] Move stack parameters for sed_ioctl to prevent oversized stack with CONFIG_KASAN Arnd Bergmann <arnd@arndb.de> - 2017-02-10 09:10 +0100
      Re: [PATCH V3 2/2] Move stack parameters for sed_ioctl to prevent  oversized stack with CONFIG_KASAN Scott Bauer <scott.bauer@intel.com> - 2017-02-10 17:10 +0100

#1577818 — [PATCH V3 2/2] Move stack parameters for sed_ioctl to prevent oversized stack with CONFIG_KASAN

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;
@@ -2355,94 +2358,57 @@ int sed_ioctl(struct opal_dev *dev, unsigned int cmd, unsigned long ptr)
 		return -ENOTSUPP;
 	}
 
-	switch (cmd) {
-	case IOC_OPAL_SAVE: {
-		struct opal_lock_unlock lk_unlk;
-
-		if (copy_from_user(&lk_unlk, arg, sizeof(lk_unlk)))
-			return -EFAULT;
-		return opal_save(dev, &lk_unlk);
-	}
-	case IOC_OPAL_LOCK_UNLOCK: {
-		struct opal_lock_unlock lk_unlk;
-
-		if (copy_from_user(&lk_unlk, arg, sizeof(lk_unlk)))
-			return -EFAULT;
-		return opal_lock_unlock(dev, &lk_unlk);
-	}
-	case IOC_OPAL_TAKE_OWNERSHIP: {
-		struct opal_key opal_key;
-
-		if (copy_from_user(&opal_key, arg, sizeof(opal_key)))
-			return -EFAULT;
-		return opal_take_ownership(dev, &opal_key);
-	}
-	case IOC_OPAL_ACTIVATE_LSP: {
-		struct opal_lr_act opal_lr_act;
-
-		if (copy_from_user(&opal_lr_act, arg, sizeof(opal_lr_act)))
-			return -EFAULT;
-		return opal_activate_lsp(dev, &opal_lr_act);
-	}
-	case IOC_OPAL_SET_PW: {
-		struct opal_new_pw opal_pw;
-
-		if (copy_from_user(&opal_pw, arg, sizeof(opal_pw)))
-			return -EFAULT;
-		return opal_set_new_pw(dev, &opal_pw);
-	}
-	case IOC_OPAL_ACTIVATE_USR: {
-		struct opal_session_info session;
-
-		if (copy_from_user(&session, arg, sizeof(session)))
-			return -EFAULT;
-		return opal_activate_user(dev, &session);
-	}
-	case IOC_OPAL_REVERT_TPR: {
-		struct opal_key opal_key;
-
-		if (copy_from_user(&opal_key, arg, sizeof(opal_key)))
-			return -EFAULT;
-		return opal_reverttper(dev, &opal_key);
-	}
-	case IOC_OPAL_LR_SETUP: {
-		struct opal_user_lr_setup lrs;
-
-		if (copy_from_user(&lrs, arg, sizeof(lrs)))
-			return -EFAULT;
-		return opal_setup_locking_range(dev, &lrs);
-	}
-	case IOC_OPAL_ADD_USR_TO_LR: {
-		struct opal_lock_unlock lk_unlk;
-
-		if (copy_from_user(&lk_unlk, arg, sizeof(lk_unlk)))
-			return -EFAULT;
-		return opal_add_user_to_lr(dev, &lk_unlk);
-	}
-	case IOC_OPAL_ENABLE_DISABLE_MBR: {
-		struct opal_mbr_data mbr;
-
-		if (copy_from_user(&mbr, arg, sizeof(mbr)))
-			return -EFAULT;
-		return opal_enable_disable_shadow_mbr(dev, &mbr);
-	}
-	case IOC_OPAL_ERASE_LR: {
-		struct opal_session_info session;
-
-		if (copy_from_user(&session, arg, sizeof(session)))
-			return -EFAULT;
-		return opal_erase_locking_range(dev, &session);
+	ioctl_ptr = kzalloc(cmd_size, GFP_KERNEL);
+	if (!ioctl_ptr)
+		return -ENOMEM;
+	if (copy_from_user(ioctl_ptr, arg, cmd_size)) {
+		ret = -EFAULT;
+		goto out;
 	}
-	case IOC_OPAL_SECURE_ERASE_LR: {
-		struct opal_session_info session;
 
-		if (copy_from_user(&session, arg, sizeof(session)))
-			return -EFAULT;
-		return opal_secure_erase_locking_range(dev, &session);
-	}
+	switch (cmd) {
+	case IOC_OPAL_SAVE:
+		ret = opal_save(dev, ioctl_ptr);
+		break;
+	case IOC_OPAL_LOCK_UNLOCK:
+		ret = opal_lock_unlock(dev, ioctl_ptr);
+		break;
+	case IOC_OPAL_TAKE_OWNERSHIP:
+		ret = opal_take_ownership(dev, ioctl_ptr);
+		break;
+	case IOC_OPAL_ACTIVATE_LSP:
+		ret = opal_activate_lsp(dev, ioctl_ptr);
+		break;
+	case IOC_OPAL_SET_PW:
+		ret = opal_set_new_pw(dev, ioctl_ptr);
+		break;
+	case IOC_OPAL_ACTIVATE_USR:
+		ret = opal_activate_user(dev, ioctl_ptr);
+		break;
+	case IOC_OPAL_REVERT_TPR:
+		ret = opal_reverttper(dev, ioctl_ptr);
+		break;
+	case IOC_OPAL_LR_SETUP:
+		ret = opal_setup_locking_range(dev, ioctl_ptr);
+		break;
+	case IOC_OPAL_ADD_USR_TO_LR:
+		ret = opal_add_user_to_lr(dev, ioctl_ptr);
+		break;
+	case IOC_OPAL_ENABLE_DISABLE_MBR:
+		ret = opal_enable_disable_shadow_mbr(dev, ioctl_ptr);
+		break;
+	case IOC_OPAL_ERASE_LR:
+		ret = opal_erase_locking_range(dev, ioctl_ptr);
+		break;
+	case IOC_OPAL_SECURE_ERASE_LR:
+		ret = opal_secure_erase_locking_range(dev, ioctl_ptr);
+		break;
 	default:
 		pr_warn("No such Opal Ioctl %u\n", cmd);
 	}
-	return -ENOTTY;
+
+out:
+	kfree(ioctl_ptr);
+	return ret;
 }
 EXPORT_SYMBOL_GPL(sed_ioctl);
-- 
2.7.4

[toc] | [next] | [standalone]

#1577908 — Re: [PATCH V3 2/2] Move stack parameters for sed_ioctl to prevent oversized stack with CONFIG_KASAN

On Thu, Feb 09, 2017 at 10:20:01AM -0700, 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()

I just went through the other threads about this issue. This approach
looks good to me.

Rafael

> 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;
> @@ -2355,94 +2358,57 @@ int sed_ioctl(struct opal_dev *dev, unsigned int cmd, unsigned long ptr)
>  		return -ENOTSUPP;
>  	}
>  
> -	switch (cmd) {
> -	case IOC_OPAL_SAVE: {
> -		struct opal_lock_unlock lk_unlk;
> -
> -		if (copy_from_user(&lk_unlk, arg, sizeof(lk_unlk)))
> -			return -EFAULT;
> -		return opal_save(dev, &lk_unlk);
> -	}
> -	case IOC_OPAL_LOCK_UNLOCK: {
> -		struct opal_lock_unlock lk_unlk;
> -
> -		if (copy_from_user(&lk_unlk, arg, sizeof(lk_unlk)))
> -			return -EFAULT;
> -		return opal_lock_unlock(dev, &lk_unlk);
> -	}
> -	case IOC_OPAL_TAKE_OWNERSHIP: {
> -		struct opal_key opal_key;
> -
> -		if (copy_from_user(&opal_key, arg, sizeof(opal_key)))
> -			return -EFAULT;
> -		return opal_take_ownership(dev, &opal_key);
> -	}
> -	case IOC_OPAL_ACTIVATE_LSP: {
> -		struct opal_lr_act opal_lr_act;
> -
> -		if (copy_from_user(&opal_lr_act, arg, sizeof(opal_lr_act)))
> -			return -EFAULT;
> -		return opal_activate_lsp(dev, &opal_lr_act);
> -	}
> -	case IOC_OPAL_SET_PW: {
> -		struct opal_new_pw opal_pw;
> -
> -		if (copy_from_user(&opal_pw, arg, sizeof(opal_pw)))
> -			return -EFAULT;
> -		return opal_set_new_pw(dev, &opal_pw);
> -	}
> -	case IOC_OPAL_ACTIVATE_USR: {
> -		struct opal_session_info session;
> -
> -		if (copy_from_user(&session, arg, sizeof(session)))
> -			return -EFAULT;
> -		return opal_activate_user(dev, &session);
> -	}
> -	case IOC_OPAL_REVERT_TPR: {
> -		struct opal_key opal_key;
> -
> -		if (copy_from_user(&opal_key, arg, sizeof(opal_key)))
> -			return -EFAULT;
> -		return opal_reverttper(dev, &opal_key);
> -	}
> -	case IOC_OPAL_LR_SETUP: {
> -		struct opal_user_lr_setup lrs;
> -
> -		if (copy_from_user(&lrs, arg, sizeof(lrs)))
> -			return -EFAULT;
> -		return opal_setup_locking_range(dev, &lrs);
> -	}
> -	case IOC_OPAL_ADD_USR_TO_LR: {
> -		struct opal_lock_unlock lk_unlk;
> -
> -		if (copy_from_user(&lk_unlk, arg, sizeof(lk_unlk)))
> -			return -EFAULT;
> -		return opal_add_user_to_lr(dev, &lk_unlk);
> -	}
> -	case IOC_OPAL_ENABLE_DISABLE_MBR: {
> -		struct opal_mbr_data mbr;
> -
> -		if (copy_from_user(&mbr, arg, sizeof(mbr)))
> -			return -EFAULT;
> -		return opal_enable_disable_shadow_mbr(dev, &mbr);
> -	}
> -	case IOC_OPAL_ERASE_LR: {
> -		struct opal_session_info session;
> -
> -		if (copy_from_user(&session, arg, sizeof(session)))
> -			return -EFAULT;
> -		return opal_erase_locking_range(dev, &session);
> +	ioctl_ptr = kzalloc(cmd_size, GFP_KERNEL);
> +	if (!ioctl_ptr)
> +		return -ENOMEM;
> +	if (copy_from_user(ioctl_ptr, arg, cmd_size)) {
> +		ret = -EFAULT;
> +		goto out;
>  	}
> -	case IOC_OPAL_SECURE_ERASE_LR: {
> -		struct opal_session_info session;
>  
> -		if (copy_from_user(&session, arg, sizeof(session)))
> -			return -EFAULT;
> -		return opal_secure_erase_locking_range(dev, &session);
> -	}
> +	switch (cmd) {
> +	case IOC_OPAL_SAVE:
> +		ret = opal_save(dev, ioctl_ptr);
> +		break;
> +	case IOC_OPAL_LOCK_UNLOCK:
> +		ret = opal_lock_unlock(dev, ioctl_ptr);
> +		break;
> +	case IOC_OPAL_TAKE_OWNERSHIP:
> +		ret = opal_take_ownership(dev, ioctl_ptr);
> +		break;
> +	case IOC_OPAL_ACTIVATE_LSP:
> +		ret = opal_activate_lsp(dev, ioctl_ptr);
> +		break;
> +	case IOC_OPAL_SET_PW:
> +		ret = opal_set_new_pw(dev, ioctl_ptr);
> +		break;
> +	case IOC_OPAL_ACTIVATE_USR:
> +		ret = opal_activate_user(dev, ioctl_ptr);
> +		break;
> +	case IOC_OPAL_REVERT_TPR:
> +		ret = opal_reverttper(dev, ioctl_ptr);
> +		break;
> +	case IOC_OPAL_LR_SETUP:
> +		ret = opal_setup_locking_range(dev, ioctl_ptr);
> +		break;
> +	case IOC_OPAL_ADD_USR_TO_LR:
> +		ret = opal_add_user_to_lr(dev, ioctl_ptr);
> +		break;
> +	case IOC_OPAL_ENABLE_DISABLE_MBR:
> +		ret = opal_enable_disable_shadow_mbr(dev, ioctl_ptr);
> +		break;
> +	case IOC_OPAL_ERASE_LR:
> +		ret = opal_erase_locking_range(dev, ioctl_ptr);
> +		break;
> +	case IOC_OPAL_SECURE_ERASE_LR:
> +		ret = opal_secure_erase_locking_range(dev, ioctl_ptr);
> +		break;
>  	default:
>  		pr_warn("No such Opal Ioctl %u\n", cmd);
>  	}
> -	return -ENOTTY;
> +
> +out:
> +	kfree(ioctl_ptr);
> +	return ret;
>  }
>  EXPORT_SYMBOL_GPL(sed_ioctl);
> -- 
> 2.7.4
> 
> 
> _______________________________________________
> Linux-nvme mailing list
> Linux-nvme@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-nvme

[toc] | [prev] | [next] | [standalone]

#1578228 — Re: [PATCH V3 2/2] Move stack parameters for sed_ioctl to prevent oversized stack with CONFIG_KASAN

On 02/09/2017 06:20 PM, 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(-)
> 

[...]

> -		if (copy_from_user(&session, arg, sizeof(session)))
> -			return -EFAULT;
> -		return opal_erase_locking_range(dev, &session);
> +	ioctl_ptr = kzalloc(cmd_size, GFP_KERNEL);
> +	if (!ioctl_ptr)
> +		return -ENOMEM;
> +	if (copy_from_user(ioctl_ptr, arg, cmd_size)) {
> +		ret = -EFAULT;
> +		goto out;
>  	}

Can't we use memdup_user() instead of kzalloc() + copy_from_user()?



-- 
Johannes Thumshirn                                          Storage
jthumshirn@suse.de                                +49 911 74053 689
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg)
Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850

[toc] | [prev] | [next] | [standalone]

#1578373 — RE: [PATCH V3 2/2] Move stack parameters for sed_ioctl to prevent oversized stack with CONFIG_KASAN

From: Johannes Thumshirn
> Sent: 10 February 2017 07:46
> On 02/09/2017 06:20 PM, Scott Bauer wrote:
> > When CONFIG_KASAN is enabled, compilation fails:

Does CONFIG_KASAN allocate guard stack space around everything that
is passed by address?
That sounds completely brain-dead.
There are a lot of functions that have an 'int *' argument to return
a single value - and are never going to do anything else.

...
> > Moved all the ioctl structures off the stack and dynamically activate
> > using _IOC_SIZE()
...
> 
> > -		if (copy_from_user(&session, arg, sizeof(session)))
> > -			return -EFAULT;
> > -		return opal_erase_locking_range(dev, &session);
> > +	ioctl_ptr = kzalloc(cmd_size, GFP_KERNEL);
> > +	if (!ioctl_ptr)
> > +		return -ENOMEM;
> > +	if (copy_from_user(ioctl_ptr, arg, cmd_size)) {
> > +		ret = -EFAULT;
> > +		goto out;
> >  	}
> 
> Can't we use memdup_user() instead of kzalloc() + copy_from_user()?

You either want the copy_from_user() or the memzero() not both.

ISTM there could be two 'library' functions, maybe:
void *get_ioctl_buf(unsigned int cmd, long arg)
to malloc the buffer, memzero/copy_from_user, returns -EFAULT if copy fails.
int put_ioctl_buf(int rval, unsigned int cmd, const void *buf)
does copy_to_user() if rval >= 0 and IOR_READ, then frees buf.
return value is rval unless the copyout fails.

	David

[toc] | [prev] | [next] | [standalone]

#1578385 — Re: [PATCH V3 2/2] Move stack parameters for sed_ioctl to prevent oversized stack with CONFIG_KASAN

On Fri, Feb 10, 2017 at 11:28 AM, David Laight <David.Laight@aculab.com> wrote:

>>
>> > -           if (copy_from_user(&session, arg, sizeof(session)))
>> > -                   return -EFAULT;
>> > -           return opal_erase_locking_range(dev, &session);
>> > +   ioctl_ptr = kzalloc(cmd_size, GFP_KERNEL);
>> > +   if (!ioctl_ptr)
>> > +           return -ENOMEM;
>> > +   if (copy_from_user(ioctl_ptr, arg, cmd_size)) {
>> > +           ret = -EFAULT;
>> > +           goto out;
>> >     }
>>
>> Can't we use memdup_user() instead of kzalloc() + copy_from_user()?
>
> You either want the copy_from_user() or the memzero() not both.
>
> ISTM there could be two 'library' functions, maybe:
> void *get_ioctl_buf(unsigned int cmd, long arg)
> to malloc the buffer, memzero/copy_from_user, returns -EFAULT if copy fails.
> int put_ioctl_buf(int rval, unsigned int cmd, const void *buf)
> does copy_to_user() if rval >= 0 and IOR_READ, then frees buf.
> return value is rval unless the copyout fails.

All the ioctls commands in this driver are IOW, and no data is passed back
to user space, so there is no need for the memzero(): we can either copy
the data from user space or we fail.

    Arnd

[toc] | [prev] | [next] | [standalone]

#1578266

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

[toc] | [prev] | [next] | [standalone]

#1578577 — Re: [PATCH V3 2/2] Move stack parameters for sed_ioctl to prevent oversized stack with CONFIG_KASAN

On Fri, Feb 10, 2017 at 09:01:23AM +0100, Arnd Bergmann wrote:
> 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.

Right now it's up to the storage driver to call into the sed-opal ioctl.
We have a function is_sed_ioctl() which is called before jumping into sed_ioctl.
So we will be weeding out any non opal ioctls before getting in there so I don't
see any overly large 16kb allocations happening.

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web