Page MenuHomeFreeBSD

tcp bblog: use correct length
ClosedPublic

Authored by tuexen on Mar 26 2024, 12:20 PM.
Tags
None
Referenced Files
Unknown Object (File)
Sun, May 19, 11:27 AM
Unknown Object (File)
Sun, May 5, 4:53 PM
Unknown Object (File)
Mar 30 2024, 5:55 PM
Unknown Object (File)
Mar 27 2024, 11:25 PM
Unknown Object (File)
Mar 27 2024, 1:58 PM
Subscribers

Details

Summary

The length of tldl_reason is TCP_LOG_REASON_LEN, not TCP_LOG_ID_LEN. No functional change intended.

Reported by: Coverity Scan
CID: 1418074
CID: 1418276

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

This revision is now accepted and ready to land.Mar 26 2024, 12:29 PM

A sidenote: nginx practice to prevent this kind of bugs is this:

strlcpy(entry->tldl_reason, "UNKNOWN", sizeof("UNKNOWN"));

I would suggest to further improve this:

#define STRWSIZE0(s) #s, sizeof(#s)
#define STRWSIZE(s) #s, sizeof(#s) - 1

to prevent probability of typing a long string twice differently.

This revision was automatically updated to reflect the committed changes.

A sidenote: nginx practice to prevent this kind of bugs is this:

strlcpy(entry->tldl_reason, "UNKNOWN", sizeof("UNKNOWN"));

I am confused: isn't the last parameter of strlcpy the size of the destination buffer, not the length of the source. So if you write something like

strlcpy(entry->tldl_reason, "This string is much longer than the buffer size of thirtytwo bytes", sizeof("This string is much longer than the buffer size of thirtytwo bytes"));

will result in a buffer overflow.
Something like

strlcpy(entry->tldl_reason, "This string is much longer than the buffer size of thirtytwo bytes", TCP_LOG_REASON_LEN);

would truncate the string correctly and assure that tldl_reason is NUL terminated.

I would suggest to further improve this:

#define STRWSIZE0(s) #s, sizeof(#s)
#define STRWSIZE(s) #s, sizeof(#s) - 1

to prevent probability of typing a long string twice differently.

I am confused: isn't the last parameter of strlcpy the size of the destination buffer, not the length of the source. So if you write something like

strlcpy(entry->tldl_reason, "This string is much longer than the buffer size of thirtytwo bytes", sizeof("This string is much longer than the buffer size of thirtytwo bytes"));

will result in a buffer overflow.

Yes, probably this particular strlcpy() was not the best example. We will write the correct length of the source string, though. For the destination size check this technique requires an additional static assert, that would at compile time check that every possible static string ever written to this destination is smaller or equal than destination.