BlueSCSI / BlueSCSI-v2

Open source, open hardware, SCSI emulator using the Pi Pico PR2040
https://bluescsi.com
GNU General Public License v3.0
225 stars 23 forks source link

SCSI_NETWORK_WIFI_CMD_SCAN_RESULTS ignores transfer size #173

Open th-otto opened 1 month ago

th-otto commented 1 month ago

In https://github.com/BlueSCSI/BlueSCSI-v2/blob/df274ea5aae5b7b2cd57a24ffafe9c981f4490a5/lib/SCSI2SD/src/firmware/network.c#L328, the size of the returned data is only checked against the size of the internal data buffer. But that ignores the size of the initiators specified transfer size in bytes 3/4 of the CDB.

I don't know if this can cause trouble, since at most 10 wifi_network_entries are returned (using 2 + 10 * 74 bytes), but IMHO this should be fixed.

BTW, is there any way to increase that number of entries (WIFI_NETWORK_LIST_ENTRY_COUNT)?

erichelgeson commented 1 month ago

Agree, should truncate the response if the host asks for a specific len.

WIFI_NETWORK_LIST_ENTRY_COUNT is 10 currently and it was just a nice round number to not take up too much RAM, as we are limited on the Pico. I suppose in apartments or the like there could be a lot more than 10 around you.

If you'd like to PR the fix, we'd love to have it!

th-otto commented 1 month ago

I understand the limitation, but the problem is that even "at home", their may be entries from your neighbours that you are not interested in (unless you know their passwords ;)

I guess its too late to change the interface, so the burden of storing/filtering the entries can be put to the host application instead?

erichelgeson commented 1 month ago

Just brainstorming.. since SCSI_NETWORK_WIFI_CMD_SCAN_RESULTS is the only place that wifi_network_list is used - instead of storing it in the wifi_network_list we can write entries directly to scsiDev.data (as is done in many places.. for better or worse) to save memory. This could be done in a compatible/non-breaking change to the existing mac wifi-da.

jcs commented 1 month ago

I think the intention was to sort by RSSI so that closer/stronger networks would appear first in the list, making it less likely that the one you care about is far down the list. But I guess I never wrote that.

https://github.com/BlueSCSI/BlueSCSI-v2/blob/df274ea5aae5b7b2cd57a24ffafe9c981f4490a5/lib/BlueSCSI_platform_RP2040/BlueSCSI_platform_network.cpp#L173

th-otto commented 1 month ago

instead of storing it in the wifi_network_list we can write entries directly to scsiDev.data

But is that safe? What if some other command is sent, while the scan is in progress (possibly from some other application).

erichelgeson commented 1 month ago

But is that safe? What if some other command is sent, while the scan is in progress (possibly from some other application).

Edit: whoops I was mistaken, you're right, we cant do this.

th-otto commented 1 month ago

PR just opened. I've no way to test it, but checked that it compiles atleast.