[PATCH] tracing/eprobe: Replace kzalloc with kmalloc

[PATCH] tracing/eprobe: Replace kzalloc with kmalloc

[Quanfa Fu]

Since this memory will be filled soon below, I feel that there is
no need for a memory of all zeros here. 'snprintf' does not return
negative num according to ISO C99, so I feel that no judgment is
needed here.

No functional change intended.

Signed-off-by: Quanfa Fu <quanfafu@gmail.com>
---
 kernel/trace/trace_eprobe.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/kernel/trace/trace_eprobe.c b/kernel/trace/trace_eprobe.c
index 352b65e2b910..cd1d271a74e7 100644
--- a/kernel/trace/trace_eprobe.c
+++ b/kernel/trace/trace_eprobe.c
@@ -917,15 +917,13 @@ static int trace_eprobe_parse_filter(struct trace_eprobe *ep, int argc, const ch
 	for (i = 0; i < argc; i++)
 		len += strlen(argv[i]) + 1;
 
-	ep->filter_str = kzalloc(len, GFP_KERNEL);
+	ep->filter_str = kmalloc(len, GFP_KERNEL);
 	if (!ep->filter_str)
 		return -ENOMEM;
 
 	p = ep->filter_str;
 	for (i = 0; i < argc; i++) {
 		ret = snprintf(p, len, "%s ", argv[i]);
-		if (ret < 0)
-			goto error;
 		if (ret > len) {
 			ret = -E2BIG;
 			goto error;
-- 
2.31.1

Re: [PATCH] tracing/eprobe: Replace kzalloc with kmalloc

[Christophe JAILLET]

Le 07/01/2023 à 04:45, Quanfa Fu a écrit :
> Since this memory will be filled soon below, I feel that there is
> no need for a memory of all zeros here. 'snprintf' does not return
> negative num according to ISO C99, so I feel that no judgment is
> needed here.
> 
> No functional change intended.
> 
> Signed-off-by: Quanfa Fu <quanfafu@gmail.com>
> ---
>   kernel/trace/trace_eprobe.c | 4 +---
>   1 file changed, 1 insertion(+), 3 deletions(-)
> 
> diff --git a/kernel/trace/trace_eprobe.c b/kernel/trace/trace_eprobe.c
> index 352b65e2b910..cd1d271a74e7 100644
> --- a/kernel/trace/trace_eprobe.c
> +++ b/kernel/trace/trace_eprobe.c
> @@ -917,15 +917,13 @@ static int trace_eprobe_parse_filter(struct trace_eprobe *ep, int argc, const ch
>   	for (i = 0; i < argc; i++)
>   		len += strlen(argv[i]) + 1;
>   
> -	ep->filter_str = kzalloc(len, GFP_KERNEL);
> +	ep->filter_str = kmalloc(len, GFP_KERNEL);
>   	if (!ep->filter_str)
>   		return -ENOMEM;
>   
>   	p = ep->filter_str;
>   	for (i = 0; i < argc; i++) {
>   		ret = snprintf(p, len, "%s ", argv[i]);
> -		if (ret < 0)
> -			goto error;
>   		if (ret > len) {

Hi,

as per [1]:
  * The return value is the number of characters which would be
  * generated for the given input, excluding the trailing null,
  * as per ISO C99.  If the return is greater than *or equal* to
  * @size, the resulting string is truncated.

So, should this test be:
     if (ret >= len)
            ~~~~


Also, isn't the p[-1] = '\0' after the loop eating the last character?
    argc = 1;
    argv[0] = "a";

    Before the loop:
    ===============
    len = 1 + 1 = 2;
    ep->filter_str = 0x00 0x00
                     ^
                     |___ p

    After the loop:
    ===============
    ep->filter_str = 0x61 0x00
                          ^
                          |___ p
    len = 1;

    After p[-1]:
    ============
    ep->filter_str = 0x00 0x00
                       ~~ ^
                          |___ p

Did I miss something obvious?
I don't know the intent here, or if it is an issue at all, but it looks odd.

CJ


[1]: https://elixir.bootlin.com/linux/v6.2-rc1/source/lib/vsprintf.c#L2925
>   			ret = -E2BIG;
>   			goto error;

Re: [PATCH] tracing/eprobe: Replace kzalloc with kmalloc

[Quanfa Fu]

On Sat, Jan 7, 2023 at 4:42 PM Christophe JAILLET
<christophe.jaillet@wanadoo.fr> wrote:
>
> Le 07/01/2023 à 04:45, Quanfa Fu a écrit :
> > Since this memory will be filled soon below, I feel that there is
> > no need for a memory of all zeros here. 'snprintf' does not return
> > negative num according to ISO C99, so I feel that no judgment is
> > needed here.
> >
> > No functional change intended.
> >
> > Signed-off-by: Quanfa Fu <quanfafu@gmail.com>
> > ---
> >   kernel/trace/trace_eprobe.c | 4 +---
> >   1 file changed, 1 insertion(+), 3 deletions(-)
> >
> > diff --git a/kernel/trace/trace_eprobe.c b/kernel/trace/trace_eprobe.c
> > index 352b65e2b910..cd1d271a74e7 100644
> > --- a/kernel/trace/trace_eprobe.c
> > +++ b/kernel/trace/trace_eprobe.c
> > @@ -917,15 +917,13 @@ static int trace_eprobe_parse_filter(struct trace_eprobe *ep, int argc, const ch
> >       for (i = 0; i < argc; i++)
> >               len += strlen(argv[i]) + 1;
> >
> > -     ep->filter_str = kzalloc(len, GFP_KERNEL);
> > +     ep->filter_str = kmalloc(len, GFP_KERNEL);
> >       if (!ep->filter_str)
> >               return -ENOMEM;
> >
> >       p = ep->filter_str;
> >       for (i = 0; i < argc; i++) {
> >               ret = snprintf(p, len, "%s ", argv[i]);
> > -             if (ret < 0)
> > -                     goto error;
> >               if (ret > len) {
>
> Hi,
>
> as per [1]:
>   * The return value is the number of characters which would be
>   * generated for the given input, excluding the trailing null,
>   * as per ISO C99.  If the return is greater than *or equal* to
>   * @size, the resulting string is truncated.
>
> So, should this test be:
>      if (ret >= len)
>             ~~~~
>
>
> Also, isn't the p[-1] = '\0' after the loop eating the last character?
>     argc = 1;
>     argv[0] = "a";
>
>     Before the loop:
>     ===============
>     len = 1 + 1 = 2;
>     ep->filter_str = 0x00 0x00
>                      ^
>                      |___ p
>
>     After the loop:
>     ===============
>     ep->filter_str = 0x61 0x00
>                           ^
>                           |___ p
>     len = 1;
>
>     After p[-1]:
>     ============
>     ep->filter_str = 0x00 0x00
>                        ~~ ^
>                           |___ p
>
> Did I miss something obvious?
> I don't know the intent here, or if it is an issue at all, but it looks odd.
>
> CJ
>
>
> [1]: https://elixir.bootlin.com/linux/v6.2-rc1/source/lib/vsprintf.c#L2925
> >                       ret = -E2BIG;
> >                       goto error;
>

I think that:

for example, argc = 2, argv = {"a", "b"};

Before the loop
===============
len = (1 + 1) + (1 + 1) = 4;
ep->filter_str = 0x00 0x00 0x00 0x00
                       ^
                        |__ p
After the loop:
===============
i = 1, snprintf write: 'a' and ' ', so ret = 2
i = 2, snprintf write: 'b' and ' ', so ret = 2
===============
ep->filter_str = 0x61 0x20 0x62 0x00    // Since the length of the
last argv is not enough to write, the space is replaced by null
                                                          ^
                                                          |__ p
p = ep->fiter_str + 2 (ret1) +2 (ret2) = ep->filter_str + 4
===============
so After p[-1] = *(ep->filter_str + 3) = '\0';
ep->filter_str = 0x61 0x20 0x62 0x00

According to ISO C99: " If the output was truncated due to this limit
hen the return value
is the number of characters (excluding the terminating null byte)
which would have been
written to the final string if enough  space  had been available"

The last snprintf will end with 'NULL', so I think p[-1] = '\0' can
also be deleted

Re: [PATCH] tracing/eprobe: Replace kzalloc with kmalloc

[Quanfa Fu]

On Sat, Jan 7, 2023 at 4:42 PM Christophe JAILLET
<christophe.jaillet@wanadoo.fr> wrote:
>
> Le 07/01/2023 à 04:45, Quanfa Fu a écrit :
> > Since this memory will be filled soon below, I feel that there is
> > no need for a memory of all zeros here. 'snprintf' does not return
> > negative num according to ISO C99, so I feel that no judgment is
> > needed here.
> >
> > No functional change intended.
> >
> > Signed-off-by: Quanfa Fu <quanfafu@gmail.com>
> > ---
> >   kernel/trace/trace_eprobe.c | 4 +---
> >   1 file changed, 1 insertion(+), 3 deletions(-)
> >
> > diff --git a/kernel/trace/trace_eprobe.c b/kernel/trace/trace_eprobe.c
> > index 352b65e2b910..cd1d271a74e7 100644
> > --- a/kernel/trace/trace_eprobe.c
> > +++ b/kernel/trace/trace_eprobe.c
> > @@ -917,15 +917,13 @@ static int trace_eprobe_parse_filter(struct trace_eprobe *ep, int argc, const ch
> >       for (i = 0; i < argc; i++)
> >               len += strlen(argv[i]) + 1;
> >
> > -     ep->filter_str = kzalloc(len, GFP_KERNEL);
> > +     ep->filter_str = kmalloc(len, GFP_KERNEL);
> >       if (!ep->filter_str)
> >               return -ENOMEM;
> >
> >       p = ep->filter_str;
> >       for (i = 0; i < argc; i++) {
> >               ret = snprintf(p, len, "%s ", argv[i]);
> > -             if (ret < 0)
> > -                     goto error;
> >               if (ret > len) {
>
> Hi,
>
> as per [1]:
>   * The return value is the number of characters which would be
>   * generated for the given input, excluding the trailing null,
>   * as per ISO C99.  If the return is greater than *or equal* to
>   * @size, the resulting string is truncated.
>
> So, should this test be:
>      if (ret >= len)
>             ~~~~
>
>
> Also, isn't the p[-1] = '\0' after the loop eating the last character?
>     argc = 1;
>     argv[0] = "a";
>
>     Before the loop:
>     ===============
>     len = 1 + 1 = 2;
>     ep->filter_str = 0x00 0x00
>                      ^
>                      |___ p
>
>     After the loop:
>     ===============
>     ep->filter_str = 0x61 0x00
>                           ^
>                           |___ p
>     len = 1;
>
>     After p[-1]:
>     ============
>     ep->filter_str = 0x00 0x00
>                        ~~ ^
>                           |___ p
>
> Did I miss something obvious?
> I don't know the intent here, or if it is an issue at all, but it looks odd.
>
> CJ
>
>
> [1]: https://elixir.bootlin.com/linux/v6.2-rc1/source/lib/vsprintf.c#L2925
> >                       ret = -E2BIG;
> >                       goto error;
>

I think that:

for example, argc = 2, argv = {"a", "b"};

Before the loop
===============
len = (1 + 1) + (1 + 1) = 4;
ep->filter_str = 0x00 0x00 0x00 0x00
                       ^
                        |__ p

After the loop:
===============
i = 1, snprintf write: 'a' and ' ', so ret = 2
i = 2, snprintf write: 'b' and ' ', so ret = 2
===============
 Since the length of the last argv is not enough
to write, the space is replaced by null

ep->filter_str = 0x61 0x20 0x62 0x00
                                                          ^
                                                          |__ p
p = ep->fiter_str + 2 (ret1) +2 (ret2) = ep->filter_str + 4
===============
so After p[-1] = *(ep->filter_str + 3) = '\0';
ep->filter_str = 0x61 0x20 0x62 0x00

According to ISO C99: " If the output was truncated due to this limit
then the return value is the number of characters (excluding the
terminating null byte) which would have been written to the final
string if enough  space  had been available"

The last snprintf will end with 'NULL', so I think p[-1] = '\0' can
also be deleted

Re: [PATCH] tracing/eprobe: Replace kzalloc with kmalloc

[Christophe JAILLET]

Le 07/01/2023 à 13:23, Quanfa Fu a écrit :
> On Sat, Jan 7, 2023 at 4:42 PM Christophe JAILLET
> <christophe.jaillet@wanadoo.fr> wrote:
>>
>> Le 07/01/2023 à 04:45, Quanfa Fu a écrit :
>>> Since this memory will be filled soon below, I feel that there is
>>> no need for a memory of all zeros here. 'snprintf' does not return
>>> negative num according to ISO C99, so I feel that no judgment is
>>> needed here.
>>>
>>> No functional change intended.
>>>
>>> Signed-off-by: Quanfa Fu <quanfafu@gmail.com>
>>> ---
>>>    kernel/trace/trace_eprobe.c | 4 +---
>>>    1 file changed, 1 insertion(+), 3 deletions(-)
>>>
>>> diff --git a/kernel/trace/trace_eprobe.c b/kernel/trace/trace_eprobe.c
>>> index 352b65e2b910..cd1d271a74e7 100644
>>> --- a/kernel/trace/trace_eprobe.c
>>> +++ b/kernel/trace/trace_eprobe.c
>>> @@ -917,15 +917,13 @@ static int trace_eprobe_parse_filter(struct trace_eprobe *ep, int argc, const ch
>>>        for (i = 0; i < argc; i++)
>>>                len += strlen(argv[i]) + 1;
>>>
>>> -     ep->filter_str = kzalloc(len, GFP_KERNEL);
>>> +     ep->filter_str = kmalloc(len, GFP_KERNEL);
>>>        if (!ep->filter_str)
>>>                return -ENOMEM;
>>>
>>>        p = ep->filter_str;
>>>        for (i = 0; i < argc; i++) {
>>>                ret = snprintf(p, len, "%s ", argv[i]);
>>> -             if (ret < 0)
>>> -                     goto error;
>>>                if (ret > len) {
>>
> 
> I think that:
> 
> for example, argc = 2, argv = {"a", "b"};
> 
> Before the loop
> ===============
> len = (1 + 1) + (1 + 1) = 4;
> ep->filter_str = 0x00 0x00 0x00 0x00
>                         ^
>                          |__ p
> 
> After the loop:
> ===============
> i = 1, snprintf write: 'a' and ' ', so ret = 2
> i = 2, snprintf write: 'b' and ' ', so ret = 2
> ===============

Ok, I missed the space after the %s.

Sorry for the noise.

>   Since the length of the last argv is not enough
> to write, the space is replaced by null
> 
> ep->filter_str = 0x61 0x20 0x62 0x00
>                                                            ^
>                                                            |__ p
> p = ep->fiter_str + 2 (ret1) +2 (ret2) = ep->filter_str + 4
> ===============
> so After p[-1] = *(ep->filter_str + 3) = '\0';
> ep->filter_str = 0x61 0x20 0x62 0x00
> 
> According to ISO C99: " If the output was truncated due to this limit
> then the return value is the number of characters (excluding the
> terminating null byte) which would have been written to the final
> string if enough  space  had been available"
> 
> The last snprintf will end with 'NULL', so I think p[-1] = '\0' can
> also be deleted

Hmm, now that I see it, I think that it is there to remove the last 
space (even if there shouldn't be any because the last snprintf will be 
truncated).

Code LGTM as-is, even if puzzling.

CJ

> 

Re: [PATCH] tracing/eprobe: Replace kzalloc with kmalloc

[Masami Hiramatsu]

On Sat,  7 Jan 2023 11:45:57 +0800
Quanfa Fu <quanfafu@gmail.com> wrote:

> Since this memory will be filled soon below, I feel that there is
> no need for a memory of all zeros here. 'snprintf' does not return
> negative num according to ISO C99, so I feel that no judgment is
> needed here.
> 
> No functional change intended.
> 

Ah, good catch. I didn't notice that snprintf() doesn't return
error code. (I confirmed that the linux internal snprintf() also
doesn't return the error code)

Acked-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>

Thank you!

> Signed-off-by: Quanfa Fu <quanfafu@gmail.com>
> ---
>  kernel/trace/trace_eprobe.c | 4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)
> 
> diff --git a/kernel/trace/trace_eprobe.c b/kernel/trace/trace_eprobe.c
> index 352b65e2b910..cd1d271a74e7 100644
> --- a/kernel/trace/trace_eprobe.c
> +++ b/kernel/trace/trace_eprobe.c
> @@ -917,15 +917,13 @@ static int trace_eprobe_parse_filter(struct trace_eprobe *ep, int argc, const ch
>  	for (i = 0; i < argc; i++)
>  		len += strlen(argv[i]) + 1;
>  
> -	ep->filter_str = kzalloc(len, GFP_KERNEL);
> +	ep->filter_str = kmalloc(len, GFP_KERNEL);
>  	if (!ep->filter_str)
>  		return -ENOMEM;
>  
>  	p = ep->filter_str;
>  	for (i = 0; i < argc; i++) {
>  		ret = snprintf(p, len, "%s ", argv[i]);
> -		if (ret < 0)
> -			goto error;
>  		if (ret > len) {
>  			ret = -E2BIG;
>  			goto error;
> -- 
> 2.31.1
> 


-- 
Masami Hiramatsu (Google) <mhiramat@kernel.org>

Re: [PATCH] tracing/eprobe: Replace kzalloc with kmalloc

[Steven Rostedt]

On Sun, 8 Jan 2023 01:01:24 +0900
Masami Hiramatsu (Google) <mhiramat@kernel.org> wrote:

> Ah, good catch. I didn't notice that snprintf() doesn't return
> error code. (I confirmed that the linux internal snprintf() also
> doesn't return the error code)
> 
> Acked-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>

I replied to to the first instance of this patch ;-)

  https://lore.kernel.org/all/20230108162222.146d136f@rorschach.local.home/

-- Steve

Re: [PATCH] tracing/eprobe: Replace kzalloc with kmalloc

[Quanfa Fu]

Thanks a lot. Learned a lot from here.

I replaced snprintf with memcpy in Patchv2


On Mon, Jan 9, 2023 at 5:22 AM Steven Rostedt <rostedt@goodmis.org> wrote:
>
> On Sat,  7 Jan 2023 11:43:35 +0800
> Quanfa Fu <quanfafu@gmail.com> wrote:
>
> > Since this memory will be filled soon below, I feel that there is
>
> kzalloc() is also used as a safety measure to make sure nothing is
> accidentally exposed. I rather keep it for safety. Just because it
> doesn't need to be here doesn't mean it should be removed. There is no
> benefit to making this kmalloc(), as this is far from a fast path.
>
> > no need for a memory of all zeros here. 'snprintf' does not return
> > negative num according to ISO C99, so I feel that no judgment is
> > needed here.
>
> Also, it's best to remove "feelings" from change logs. Code updates are
> not made due to how one feels about something (at least it shouldn't
> be), but about having technical reasons for doing so. I do agree
> there's no reason to check snprintf() from returning negative, as
> looking at its implementation, there is no negative return. Thus, the
> change log should be:
>
>  "No need to check for negative return value from snprintf() as the
>  code does not return negative values."
>
> >
> > No functional change intended.
>
> And this does have functional changes. If the output of a compiler is
> different for a function, then that's a functional change. What we
> consider non functional changes is if functions get moved around, or
> possibly code in a function is moved into a helper function where the
> compiler *should* end up with the same assembly.
>
> Changing what malloc is called is definitely a functional change.
>
> >
> > Signed-off-by: Quanfa Fu <quanfafu@gmail.com>
> > ---
> >  kernel/trace/trace_eprobe.c | 4 +---
> >  1 file changed, 1 insertion(+), 3 deletions(-)
> >
> > diff --git a/kernel/trace/trace_eprobe.c b/kernel/trace/trace_eprobe.c
> > index 352b65e2b910..cd1d271a74e7 100644
> > --- a/kernel/trace/trace_eprobe.c
> > +++ b/kernel/trace/trace_eprobe.c
> > @@ -917,15 +917,13 @@ static int trace_eprobe_parse_filter(struct trace_eprobe *ep, int argc, const ch
> >       for (i = 0; i < argc; i++)
> >               len += strlen(argv[i]) + 1;
> >
> > -     ep->filter_str = kzalloc(len, GFP_KERNEL);
> > +     ep->filter_str = kmalloc(len, GFP_KERNEL);
> >       if (!ep->filter_str)
> >               return -ENOMEM;
> >
> >       p = ep->filter_str;
> >       for (i = 0; i < argc; i++) {
> >               ret = snprintf(p, len, "%s ", argv[i]);
>
> I wonder if this should be a strncat() instead?
>
> > -             if (ret < 0)
> > -                     goto error;
> >               if (ret > len) {
> >                       ret = -E2BIG;
> >                       goto error;
>
>         for (i = 0; i < arcc, i++)
>                 strncat(ep->filter_str, argv[i], len);
>
> I mean, before this code we have that loop already determining what len
> is, do we really need to check if it is going to be -E2BIG?
>
> -- Steve

Re: [PATCH] tracing/eprobe: Replace kzalloc with kmalloc

[Steven Rostedt]

On Sat,  7 Jan 2023 11:43:35 +0800
Quanfa Fu <quanfafu@gmail.com> wrote:

> Since this memory will be filled soon below, I feel that there is

kzalloc() is also used as a safety measure to make sure nothing is
accidentally exposed. I rather keep it for safety. Just because it
doesn't need to be here doesn't mean it should be removed. There is no
benefit to making this kmalloc(), as this is far from a fast path.

> no need for a memory of all zeros here. 'snprintf' does not return
> negative num according to ISO C99, so I feel that no judgment is
> needed here.

Also, it's best to remove "feelings" from change logs. Code updates are
not made due to how one feels about something (at least it shouldn't
be), but about having technical reasons for doing so. I do agree
there's no reason to check snprintf() from returning negative, as
looking at its implementation, there is no negative return. Thus, the
change log should be:

 "No need to check for negative return value from snprintf() as the
 code does not return negative values."

> 
> No functional change intended.

And this does have functional changes. If the output of a compiler is
different for a function, then that's a functional change. What we
consider non functional changes is if functions get moved around, or
possibly code in a function is moved into a helper function where the
compiler *should* end up with the same assembly.

Changing what malloc is called is definitely a functional change.

> 
> Signed-off-by: Quanfa Fu <quanfafu@gmail.com>
> ---
>  kernel/trace/trace_eprobe.c | 4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)
> 
> diff --git a/kernel/trace/trace_eprobe.c b/kernel/trace/trace_eprobe.c
> index 352b65e2b910..cd1d271a74e7 100644
> --- a/kernel/trace/trace_eprobe.c
> +++ b/kernel/trace/trace_eprobe.c
> @@ -917,15 +917,13 @@ static int trace_eprobe_parse_filter(struct trace_eprobe *ep, int argc, const ch
>  	for (i = 0; i < argc; i++)
>  		len += strlen(argv[i]) + 1;
>  
> -	ep->filter_str = kzalloc(len, GFP_KERNEL);
> +	ep->filter_str = kmalloc(len, GFP_KERNEL);
>  	if (!ep->filter_str)
>  		return -ENOMEM;
>  
>  	p = ep->filter_str;
>  	for (i = 0; i < argc; i++) {
>  		ret = snprintf(p, len, "%s ", argv[i]);

I wonder if this should be a strncat() instead?

> -		if (ret < 0)
> -			goto error;
>  		if (ret > len) {
>  			ret = -E2BIG;
>  			goto error;

	for (i = 0; i < arcc, i++)
		strncat(ep->filter_str, argv[i], len);

I mean, before this code we have that loop already determining what len
is, do we really need to check if it is going to be -E2BIG?

-- Steve

[PATCH] tracing/eprobe: Replace kzalloc with kmalloc

[Quanfa Fu]

Since this memory will be filled soon below, I feel that there is
no need for a memory of all zeros here. 'snprintf' does not return
negative num according to ISO C99, so I feel that no judgment is
needed here.

No functional change intended.

Signed-off-by: Quanfa Fu <quanfafu@gmail.com>
---
 kernel/trace/trace_eprobe.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/kernel/trace/trace_eprobe.c b/kernel/trace/trace_eprobe.c
index 352b65e2b910..cd1d271a74e7 100644
--- a/kernel/trace/trace_eprobe.c
+++ b/kernel/trace/trace_eprobe.c
@@ -917,15 +917,13 @@ static int trace_eprobe_parse_filter(struct trace_eprobe *ep, int argc, const ch
 	for (i = 0; i < argc; i++)
 		len += strlen(argv[i]) + 1;
 
-	ep->filter_str = kzalloc(len, GFP_KERNEL);
+	ep->filter_str = kmalloc(len, GFP_KERNEL);
 	if (!ep->filter_str)
 		return -ENOMEM;
 
 	p = ep->filter_str;
 	for (i = 0; i < argc; i++) {
 		ret = snprintf(p, len, "%s ", argv[i]);
-		if (ret < 0)
-			goto error;
 		if (ret > len) {
 			ret = -E2BIG;
 			goto error;
-- 
2.31.1