CA-220442: Check length of socket name before copying #202
Conversation
| @@ -187,6 +187,13 @@ tap_ctl_connect(const char *name, int *sfd) | |||
|
|
|||
| memset(&saddr, 0, sizeof(saddr)); | |||
| saddr.sun_family = AF_UNIX; | |||
|
|
|||
| if (unlikely(strlen(name) > sizeof(saddr.sun_path) - 1)) { | |||
There was a problem hiding this comment.
This line should be more readable by adding parenthesis around sizeof and -1 instead of relying on operator precedence.
I do see your point in raising an error instead of truncating and keep going
There was a problem hiding this comment.
I agree changing 'sizeof(saddr.sun_path) - 1" to '(sizeof(saddr.sun_path) - 1)' would make it a bit more readable
There was a problem hiding this comment.
This is a random line taken from the linux kernel
if (unlikely(!bvl && (gfp_mask & __GFP_DIRECT_RECLAIM)))
I think whoever wrote the block layer knows C enough, nevertheless they felt the need to
make it readable.
There is a difference between readable and understanble
There was a problem hiding this comment.
Or even pull the maximum allowed size out as a constant and use that, no parens required.
There was a problem hiding this comment.
apples and oranges...
kostaslambda
force-pushed
the
CA-220442
branch
from
9d134bc
to
04a60a4
Compare
|
Don't quite understand the apple and oranges comment. I am getting the commit as is, I think we spent far too much time on this one. |
'struct sockaddr_un' member 'sun_path' is a fixed size, 108 byte array. Check that 'name' is at maximum 107 characters long before copying. Signed-off-by: Kostas Ladopoulos <konstantinos.ladopoulos@citrix.com>
kostaslambda
force-pushed
the
CA-220442
branch
from
04a60a4
to
4d5da89
Compare
'struct sockaddr_un' member 'sun_path' is a fixed size, 108 byte array.
Check that 'name' is at maximum 107 characters long before copying.
Signed-off-by: Kostas Ladopoulos konstantinos.ladopoulos@citrix.com