diff mbox series

[v2,2/5] params: Do not go over the limit when getting the string length

Message ID 20231002124856.2455696-3-andriy.shevchenko@linux.intel.com
State New
Headers
Series params: harden string ops and allocatio ops |

Commit Message

Andy Shevchenko Oct. 2, 2023, 12:48 p.m. UTC 
  We can use strnlen() even on early stages and it prevents from
going over the string boundaries in case it's already too long.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 kernel/params.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

Comments

Kees Cook Oct. 2, 2023, 4:57 p.m. UTC | #1 
On Mon, Oct 02, 2023 at 03:48:53PM +0300, Andy Shevchenko wrote:
> We can use strnlen() even on early stages and it prevents from
> going over the string boundaries in case it's already too long.

It makes sense to avoid calling strlen() multiple times. I don't have
much opinion one way or another about using strnlen() here, since we
know the string will be terminated.

-Kees

> 
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> ---
>  kernel/params.c | 9 ++++++---
>  1 file changed, 6 insertions(+), 3 deletions(-)
> 
> diff --git a/kernel/params.c b/kernel/params.c
> index 626fa8265932..f8e3c4139854 100644
> --- a/kernel/params.c
> +++ b/kernel/params.c
> @@ -260,7 +260,10 @@ EXPORT_SYMBOL_GPL(param_set_uint_minmax);
>  
>  int param_set_charp(const char *val, const struct kernel_param *kp)
>  {
> -	if (strlen(val) > 1024) {
> +	size_t len, maxlen = 1024;
> +
> +	len = strnlen(val, maxlen + 1);
> +	if (len == maxlen + 1) {
>  		pr_err("%s: string parameter too long\n", kp->name);
>  		return -ENOSPC;
>  	}
> @@ -270,7 +273,7 @@ int param_set_charp(const char *val, const struct kernel_param *kp)
>  	/* This is a hack.  We can't kmalloc in early boot, and we
>  	 * don't need to; this mangled commandline is preserved. */
>  	if (slab_is_available()) {
> -		*(char **)kp->arg = kmalloc_parameter(strlen(val)+1);
> +		*(char **)kp->arg = kmalloc_parameter(len + 1);
>  		if (!*(char **)kp->arg)
>  			return -ENOMEM;
>  		strcpy(*(char **)kp->arg, val);
> @@ -508,7 +511,7 @@ int param_set_copystring(const char *val, const struct kernel_param *kp)
>  {
>  	const struct kparam_string *kps = kp->str;
>  
> -	if (strlen(val)+1 > kps->maxlen) {
> +	if (strnlen(val, kps->maxlen) == kps->maxlen) {
>  		pr_err("%s: string doesn't fit in %u chars.\n",
>  		       kp->name, kps->maxlen-1);
>  		return -ENOSPC;
> -- 
> 2.40.0.1.gaa8946217a0b
>
diff mbox series

Patch

diff --git a/kernel/params.c b/kernel/params.c
index 626fa8265932..f8e3c4139854 100644
--- a/kernel/params.c
+++ b/kernel/params.c
@@ -260,7 +260,10 @@  EXPORT_SYMBOL_GPL(param_set_uint_minmax);
 
 int param_set_charp(const char *val, const struct kernel_param *kp)
 {
-	if (strlen(val) > 1024) {
+	size_t len, maxlen = 1024;
+
+	len = strnlen(val, maxlen + 1);
+	if (len == maxlen + 1) {
 		pr_err("%s: string parameter too long\n", kp->name);
 		return -ENOSPC;
 	}
@@ -270,7 +273,7 @@  int param_set_charp(const char *val, const struct kernel_param *kp)
 	/* This is a hack.  We can't kmalloc in early boot, and we
 	 * don't need to; this mangled commandline is preserved. */
 	if (slab_is_available()) {
-		*(char **)kp->arg = kmalloc_parameter(strlen(val)+1);
+		*(char **)kp->arg = kmalloc_parameter(len + 1);
 		if (!*(char **)kp->arg)
 			return -ENOMEM;
 		strcpy(*(char **)kp->arg, val);
@@ -508,7 +511,7 @@  int param_set_copystring(const char *val, const struct kernel_param *kp)
 {
 	const struct kparam_string *kps = kp->str;
 
-	if (strlen(val)+1 > kps->maxlen) {
+	if (strnlen(val, kps->maxlen) == kps->maxlen) {
 		pr_err("%s: string doesn't fit in %u chars.\n",
 		       kp->name, kps->maxlen-1);
 		return -ENOSPC;