Fix process_open_sockets type error on darwin #6546
Conversation
| @@ -147,9 +147,14 @@ void genSocketDescriptor(int pid, int descriptor, QueryData& results) { | |||
| if (si.psi.soi_family == AF_INET || si.psi.soi_family == AF_INET6) { | |||
| Row r; | |||
|
|
|||
| // sqlite is signed 64bit, but soi_so is unsigned. If it's above | |||
| // LLONG_MAX, may as well drop it. That's all we did prior. | |||
| if (si.psi.soi_so <= LLONG_MAX) { | |||
There was a problem hiding this comment.
Might be cleaner to put this into virtual.cpp
There was a problem hiding this comment.
I am hesitant to silence this warning without understanding/documenting the scenario. Under what circumstances is this not the case?
There was a problem hiding this comment.
I think that's a good argument to not do something in dynamic_table_row.cpp. But I think we should do something here. As is, this is typical:
osquery> select * from process_open_sockets limit 10;
I0707 15:40:51.245728 278652352 dynamic_table_row.cpp:128] Error casting socket (12188630713064942411) to BIGINT. osquery::ConversionError[1] (Value read is out of the range of representable values by an int. stoll: out of range)
I0707 15:40:51.245841 278652352 dynamic_table_row.cpp:128] Error casting socket (12188630712930999787) to BIGINT. osquery::ConversionError[1] (Value read is out of the range of representable values by an int. stoll: out of range)
+-----+----+--------+--------+----------+---------------+----------------+------------+-------------+------+-------+
| pid | fd | socket | family | protocol | local_address | remote_address | local_port | remote_port | path | state |
+-----+----+--------+--------+----------+---------------+----------------+------------+-------------+------+-------+
| 102 | 7 | | 2 | 17 | 0.0.0.0 | 0.0.0.0 | 0 | 0 | | |
| 102 | | 16 | 0 | 0 | | | 0 | 0 | | |
| 276 | | 3 | 0 | 0 | | | 0 | 0 | | |
| 286 | 3 | | 2 | 17 | 0.0.0.0 | 0.0.0.0 | 0 | 0 | | |
| 286 | | 5 | 0 | 0 | | | 0 | 0 | | |
| 286 | | 6 | 0 | 0 | | | 0 | 0 | | |
| 286 | | 7 | 0 | 0 | | | 0 | 0 | | |
| 286 | | 8 | 0 | 0 | | | 0 | 0 | | |
| 286 | | 9 | 0 | 0 | | | 0 | 0 | | |
| 290 | | 3 | 0 | 0 | | | 0 | 0 | | |
+-----+----+--------+--------+----------+---------------+----------------+------------+-------------+------+-------+
I can't find much for docs about what soi_so is, other than the bsd headers declare it as uint64. Maybe the family and protocol mean something to someone.
There was a problem hiding this comment.
Based on @Smjert's analysis, that soi_so is an address when protocol != 0 I think we should opt to not report anything in this column. I think not reporting anything is better than now and we can leave a note in source code that we are unsure how to find the socket or file descriptor.
|
Ping, what is the next step here? Any thoughts on how we can find the root cause of the large integer values? |
I did some digging before, and I don't have a simple answer. All I found was the header declaration. Maybe there's a more clever way to handle uint64 -> int64. We could convert the high bit to negation... |
|
Converting a uint64_t to int64_t via casting it's perfectly fine: Now the question here is more: are there meaningful values of As for EDIT: |
| @@ -147,9 +147,14 @@ void genSocketDescriptor(int pid, int descriptor, QueryData& results) { | |||
| if (si.psi.soi_family == AF_INET || si.psi.soi_family == AF_INET6) { | |||
| Row r; | |||
|
|
|||
| // sqlite is signed 64bit, but soi_so is unsigned. If it's above | |||
| // LLONG_MAX, may as well drop it. That's all we did prior. | |||
| if (si.psi.soi_so <= LLONG_MAX) { | |||
There was a problem hiding this comment.
Based on @Smjert's analysis, that soi_so is an address when protocol != 0 I think we should opt to not report anything in this column. I think not reporting anything is better than now and we can leave a note in source code that we are unsure how to find the socket or file descriptor.
On darwin, `soi_so` is an unsigned 64bit number. sqlite, however, is signed. Current behavior is to emit a warning, and drop the column. It's not clear we have a good fix for the type mismatch, so comment and drop it in the virtual table instead. Relates to: osquery#6319
352eba9 to
1b05bc6
Compare
On darwin,
soi_sois an unsigned 64bit number. sqlite, however, issigned. Current behavior is to emit a warning, and drop the column.
It's not clear we have a good fix for the type mismatch, so comment and
drop it in the virtual table instead.
Relates to: #6319