Re: [PATCH 16/20] tracing/probe: Manual replacement of the deprecated strlcpy() with return values

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 





Le lun. 22 févr. 2021 à 18:49, Steven Rostedt <rostedt@xxxxxxxxxxx> a écrit :
> -     if (unlikely(!maxlen))
> -             return -ENOMEM;

Don't remove the above. You just broke the else side.

> -
> -     if (addr == FETCH_TOKEN_COMM)
> -             ret = strlcpy(dst, current->comm, maxlen);
> -     else
> +     if (addr == FETCH_TOKEN_COMM) {
> +             ret = strscpy(dst, current->comm, maxlen);
> +             if (ret == -E2BIG)
> +                     return -ENOMEM;

I'm not sure the above is what we want. current->comm is always nul
terminated, and not only that, it will never be bigger than TASK_COMM_LEN.
If the "dst" location is smaller than comm (maxlen < TASK_COMM_LEN), it is
still OK to copy a partial string. It should not return -ENOMEM which looks
to be what happens with this patch.

In other words, it looks like this patch breaks the current code in more
ways than one.

-- Steve

Hello,

Mhhh, *I think* that I had an issue during rebase, I don't remember to have removed the "  if (unlikely(!maxlen))"  (sorry for that).
Well, strscpy always returns a truncated string even in case of possible overflow, the function copies what it can in "dst", it will just return -E2BIG when
it does not fit or when "count" has a bad value (zero or > INT_MAX). We have just to make a difference between "-E2BIG, data has been copied to dst and it is truncated" and "-E2BIG, possible wrong size passed as argument".

I agree that it needs at least to work like before, and I think we can preserve the old behaviour even with strscpy (we just need to adapt the error handling accordingly).
I will fix this in v2.

Thanks,
Romain
 

[Index of Archives]     [Linux Samsung SoC]     [Linux Rockchip SoC]     [Linux Actions SoC]     [Linux for Synopsys ARC Processors]     [Linux NFS]     [Linux NILFS]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]


  Powered by Linux