4:08 a.m.
Hi Mat,
Yes, in the very rare case of a programmer error during development. Are
we that worried about the microsecond-level efficiency of a very rarely
executed error path?
Not really about performance, no. But readability wise its far easier
to just error out early than tear things down.
In this particular case it doesn't matter much. And by the way
family_alloc result isn't checked inside l_genl_new...
If family_alloc uses strcpy, then you depend on reviewers giving
family_alloc the same level of scrutiny they do strcpy. You can't depend
on static analysis to catch it in the future after flagging it as "ok" now.
If family_alloc uses l_strlcpy, then the common case gets an extra call
to strlen.
Fundamentally this feels like a hammer seeking a nail. Can we make the
tool work for us and not the other way around? If the tool can't figure
out that the string length has been sanitized previously, what good is it?
>
> why is it incorrect? We're getting the name from the kernel and
> copying it into our own data structure. If the kernel uses
> GENL_NAMSIZ max-size buffers, then using strncpy is just fine, no?
It's incorrect because you aren't guaranteed a null-terminated
destination buffer. It would be correct to use strncpy with
GENL_NAMSIZ-1 and write a '\0' to to the end of the buffer if it's not
known to be zeroed.
So the kernel is misbehaving...
>
> At least I don't see how strlcpy is saving you here. If you want to
> be paranoid, you'd be taking the tlv length value into account as well.
It's making the current code work as intended.
If you don't trust the kernel and we really want to be paranoid, we
should use the length value returned by l_genl_attr_next and check that
its <= GENL_NAMESIZ, etc.
Regards,
-Denis