On 2015-04-28 18:31, Felipe Balbi wrote: > On Tue, Apr 28, 2015 at 06:15:51PM +0200, Krzysztof Opasiak wrote: >> On 04/28/2015 05:59 PM, Stefan Agner wrote: >> >On 2015-04-28 17:00, Krzysztof Opasiak wrote: >> >>Hi Stefan, >> >> >> >>On 04/28/2015 01:51 PM, Stefan Agner wrote: >> >>>MAC addresses can be written without leading zeros. A popular >> >>>example is libc's ether_ntoa_r function which creates such >> >>>MAC addresses. >> >>> >> >>>Example: >> >>>00:14:3d:0f:ff:fe can be written as 0:14:3d:f:ff:fe >> >>> >> >>>Additionally, get_ether_addr potentially parsed past the end >> >>>of the user provided string. Use the opportunity and fix the >> >>>function to never parse beyond the end of the string while >> >>>allowing MAC addresses with and without leading zeros. >> >>> >> >>>Signed-off-by: Stefan Agner <stefan@xxxxxxxx> >> >> (...) >> >> >> >> >>First of all thank you for your remark about reading past the buffer >> >>in my version of this patch. I tried to make my change as little as >> >>possible and didn't catch that this function is broken from beginning. >> >> >> >>This one is definitely better in this case but I'm afraid that it >> >>won't be able to correctly interpret MAC address written without >> >>separators while original version works fine for it. >> > >> >That is true, we need separators to be able to parse the address now. Do >> >we need to support the format without separators? The sysfs >> >documentation is not very clear about the supported format (e.g. >> >Documentation/ABI/testing/configfs-usb-gadget-ecm). >> > >> >Having variable length of the individual bytes and no separators for the >> >individual bytes somehow contradicts. We can of course just assume that >> >we have 2 characters when using no separators, its just somewhat >> >irregular... >> >> Let's check what is currently supported: >> >> 1) 00:14:3d:0f:ff:fe (no skipped 0, semicolons) >> 2) 00.14.3d.0f.ff.fe (no skipped 0, dots) >> 3) 00143d0ffffe (no skipped 0, no separator) >> >> What we are actually trying to do is adding a new format: >> >> 4) 0:14:3d:f:ff:fe (skipped leading 0, semicolons) >> 5) 0.14.3d.f.ff.fe (skipped leading 0, dots) >> >> In my humble opinion adding a new format in this case is a very good idea >> but it should not affect any other format that is currently supported by >> this function > > agreed :-) I would have traded the simpler and more robust code with format 3 :-) However, when preserving format 3, my parsing approach is probably not suited, since it requires the separator to increment the destination arrays index. I will see what I can do. Btw, one other odd format which is used by module parameters is coma separated bytes with hexadecimal indicators: 0x00,0x04,0x9f,0x01,0x30,0xe0 Which allows to use the module_param_array. But probably definitely out of scope here. -- Stefan -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
