Messages in this thread | | | Subject | Re: [PATCH 1/1] staging: rtl8723bs: make memcmp() calls consistent | From | Hans de Goede <> | Date | Wed, 13 Dec 2017 15:49:12 +0100 |
| |
Hi,
On 13-12-17 12:47, Greg Kroah-Hartman wrote: > On Sun, Dec 10, 2017 at 08:35:12PM +0100, Nicolas Iooss wrote: >> rtw_pm_set() uses memcmp() with 5-chars strings and a length of 4 when >> parsing extra, and then parses extra+4 as an int: >> >> if (!memcmp(extra, "lps =", 4)) { >> sscanf(extra+4, "%u", &mode); >> /* ... */ >> } else if (!memcmp(extra, "ips =", 4)) { >> sscanf(extra+4, "%u", &mode); >> >> The space between the key ("lps" and "ips") and the equal sign seems >> suspicious. Remove it in order to make the calls to memcmp() consistent. > > But you now just changing the parsing logic. What broke because of > this? Did you test this codepath with your patch? > > I'm not disagreeing that this code seems really odd, but it must be > working as-is for someone, to change this logic will break their system > :(
I don't think this code can work actually, for the memcmp to match the extra argument must start with e.g. : "lps =" but then mode gets read as: sscanf(extra+4, "%u", &mode);, with extra + 4 pointing at the "=" in the extra arg, so sscanf will stop right away and store 0 in mode.
The rtw_pm_set function is only used in the rtw_private_handler[] function pointer array, which itself is used here:
struct iw_handler_def rtw_handlers_def = { .standard = rtw_handlers, .num_standard = ARRAY_SIZE(rtw_handlers), #if defined(CONFIG_WEXT_PRIV) .private = rtw_private_handler, .private_args = (struct iw_priv_args *)rtw_private_args, .num_private = ARRAY_SIZE(rtw_private_handler), .num_private_args = ARRAY_SIZE(rtw_private_args), #endif .get_wireless_stats = rtw_get_wireless_stats, };
So this is for a private extension to the iw interface. I think that as part of the cleanup of this driver in staging we should just remove all private extensions, which will nicely fix the broken function by simply removing it :)
Regards,
Hans
| |